The pathname wasn't previously filled when the getFile() method was called with openFile = false.
We are caching FileEntry-s in ParsedAST::Includes in clangd and this caused the problem.
This fixes an internal test failure in clangd - ClangdTests.GoToInclude.All
rdar://47536127
Differential Revision: https://reviews.llvm.org/D58213
llvm-svn: 354075
Summary:
r347205 fixed a bug in FileManager: first calling
getFile(shouldOpen=false) and then getFile(shouldOpen=true) results in
the file not being open.
Unfortunately, some code was (inadvertently?) relying on this bug: when
building with a PCH, the file entries are obtained first by passing
shouldOpen=false, and then later shouldOpen=true, without any intention
of reading them. After r347205, they do get unneccesarily opened.
Aside from extra operations, this means they need to be closed. Normally
files are closed when their contents are read. As these files are never
read, they stay open until clang exits. On platforms with a low
open-files limit (e.g. Mac), this can lead to spurious file-not-found
errors when building large projects with PCH enabled, e.g.
https://bugs.chromium.org/p/chromium/issues/detail?id=924225
Fixing the callsites to pass shouldOpen=false when the file won't be
read is not quite trivial (that info isn't available at the direct
callsite), and passing shouldOpen=false is a performance regression (it
results in open+fstat pairs being replaced by stat+open).
So an ideal fix is going to be a little risky and we need some fix soon
(especially for the llvm 8 branch).
The problem addressed by r347205 is rare and has only been observed in
clangd. It was present in llvm-7, so we can live with it for now.
Reviewers: bkramer, thakis
Subscribers: ilya-biryukov, ioeric, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D57165
llvm-svn: 352079
to reflect the new license.
We understand that people may be surprised that we're moving the header
entirely to discuss the new license. We checked this carefully with the
Foundation's lawyer and we believe this is the correct approach.
Essentially, all code in the project is now made available by the LLVM
project under our new license, so you will see that the license headers
include that license only. Some of our contributors have contributed
code under our old license, and accordingly, we have retained a copy of
our old license notice in the top-level files in each project and
repository.
llvm-svn: 351636
removed
Stat cache chaining was implemented for a StatListener in the PTH writer so that
it could write out the stat information to PTH. r348266 removed support for PTH,
and it doesn't seem like there are other uses of stat cache chaining. We can
remove the chaining support.
Differential Revision: https://reviews.llvm.org/D55455
llvm-svn: 349942
Summary:
Absolute path information for virtual files were missing even if we
have already stat'd the files. This patch puts that information for virtual
files that can succesffully be stat'd.
Reviewers: ilya-biryukov
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D55054
llvm-svn: 348006
Summary:
Old behavior is to just return the cached entry regardless of opened-ness.
That feels buggy (though I guess nobody ever actually needed this).
This came up in the context of clangd+clang-tidy integration: we're
going to getFile(open=false) to replay preprocessor actions obscured by
the preamble, but the compilation may subsequently getFile(open=true)
for non-preamble includes.
Reviewers: ilya-biryukov
Subscribers: ioeric, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54691
llvm-svn: 347205
This patch moves the virtual file system form clang to llvm so it can be
used by more projects.
Concretely the patch:
- Moves VirtualFileSystem.{h|cpp} from clang/Basic to llvm/Support.
- Moves the corresponding unit test from clang to llvm.
- Moves the vfs namespace from clang::vfs to llvm::vfs.
- Formats the lines affected by this change, mostly this is the result of
the added llvm namespace.
RFC on the mailing list:
http://lists.llvm.org/pipermail/llvm-dev/2018-October/126657.html
Differential revision: https://reviews.llvm.org/D52783
llvm-svn: 344140
Revert to the original behavior: only calculate real file path when
file is opened and avoid using InterndPath for real path calculation.
llvm-svn: 340602
Summary:
This partially rolls back the change in D48903:
89aa7f45a1 (diff-0025af005307891b5429b6a834823d5eR318)
`real_path` can be very expensive on real file systems, and calling it on each
opened file can slow down the compilation. This also slows down deserialized
ASTs for which real paths need to be recalculated for each input files again.
For clangd code completion latency (using preamble):
Before
{F7039629}
After
{F7039630}
Reviewers: ilya-biryukov, simark
Reviewed By: ilya-biryukov
Subscribers: kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D51159
llvm-svn: 340598
Summary:
InMemoryFileSystem::status behaves differently than
RealFileSystem::status. The Name contained in the Status returned by
RealFileSystem::status will be the path as requested by the caller,
whereas InMemoryFileSystem::status returns the normalized path.
For example, when requested the status for "../src/first.h",
RealFileSystem returns a Status with "../src/first.h" as the Name.
InMemoryFileSystem returns "/absolute/path/to/src/first.h".
The reason for this change is that I want to make a unit test in the
clangd testsuite (where we use an InMemoryFileSystem) to reproduce a
bug I get with the clangd program (where a RealFileSystem is used).
This difference in behavior "hides" the bug in the unit test version.
An indirect impact of this change is that a -Wnonportable-include-path
warning is now emitted in test PCH/case-insensitive-include.c. This is
because the real path of the included file (with the wrong case) was not
available previously, whereas it is now.
Reviewers: malaperle, ilya-biryukov, bkramer
Reviewed By: ilya-biryukov
Subscribers: eric_niebler, malaperle, omtcyfz, hokein, bkramer, ilya-biryukov, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D48903
llvm-svn: 339063
Summary:
InMemoryFileSystem::status behaves differently than
RealFileSystem::status. The Name contained in the Status returned by
RealFileSystem::status will be the path as requested by the caller,
whereas InMemoryFileSystem::status returns the normalized path.
For example, when requested the status for "../src/first.h",
RealFileSystem returns a Status with "../src/first.h" as the Name.
InMemoryFileSystem returns "/absolute/path/to/src/first.h".
The reason for this change is that I want to make a unit test in the
clangd testsuite (where we use an InMemoryFileSystem) to reproduce a
bug I get with the clangd program (where a RealFileSystem is used).
This difference in behavior "hides" the bug in the unit test version.
Reviewers: malaperle, ilya-biryukov, bkramer
Subscribers: cfe-commits, ioeric, ilya-biryukov, bkramer, hokein, omtcyfz
Differential Revision: https://reviews.llvm.org/D48903
llvm-svn: 338057
Summary:
- add comments clarifying semantics
- Status::copyWithNewName(Status, Name) --> instance method
- Status::copyWithNewName(fs::file_status, Name) --> constructor (it's not a copy)
- File::getName() -> getRealPath(), reflecting its actual behavior/function
and stop returning status().getName() in the base class (callers can do this
fallback if they want to, it complicates the contracts).
This is mostly NFC, but the behavior of File::getName() affects FileManager's
FileEntry::tryGetRealPathName(), which now fails in more cases:
- non-real file cases
- real-file cases where the underlying vfs::File was opened in a way that
doesn't call realpath().
(In these cases we don't know a distinct real name, so in principle it seems OK)
Reviewers: klimek
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D49724
llvm-svn: 337834
This reverts commit r336807. This breaks users of
ClangTool::mapVirtualFile. Will try to investigate a fix. See also the
discussion on https://reviews.llvm.org/D48903
llvm-svn: 336831
Summary:
InMemoryFileSystem::status behaves differently than
RealFileSystem::status. The Name contained in the Status returned by
RealFileSystem::status will be the path as requested by the caller,
whereas InMemoryFileSystem::status returns the normalized path.
For example, when requested the status for "../src/first.h",
RealFileSystem returns a Status with "../src/first.h" as the Name.
InMemoryFileSystem returns "/absolute/path/to/src/first.h".
The reason for this change is that I want to make a unit test in the
clangd testsuite (where we use an InMemoryFileSystem) to reproduce a
bug I get with the clangd program (where a RealFileSystem is used).
This difference in behavior "hides" the bug in the unit test version.
In general, I guess it's good if InMemoryFileSystem works as much as
possible like RealFileSystem.
Doing so made the FileEntry::RealPathName value (assigned in
FileManager::getFile) wrong when using the InMemoryFileSystem. That's
because it assumes that vfs::File::getName will always return the real
path. I changed to to use FileSystem::getRealPath instead.
Subscribers: ilya-biryukov, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D48903
llvm-svn: 336807
Do not memory map the main file if the flag UserFilesAreVolatile is set to true
in ASTUnit when calling FileSystem::getBufferForFile.
Differential Revision: https://reviews.llvm.org/D47460
llvm-svn: 334070
This is similar to the LLVM change https://reviews.llvm.org/D46290.
We've been running doxygen with the autobrief option for a couple of
years now. This makes the \brief markers into our comments
redundant. Since they are a visual distraction and we don't want to
encourage more \brief markers in new code either, this patch removes
them all.
Patch produced by
for i in $(git grep -l '\@brief'); do perl -pi -e 's/\@brief //g' $i & done
for i in $(git grep -l '\\brief'); do perl -pi -e 's/\\brief //g' $i & done
Differential Revision: https://reviews.llvm.org/D46320
llvm-svn: 331834
LLVM_ON_WIN32 is set exactly with MSVC and MinGW (but not Cygwin) in
HandleLLVMOptions.cmake, which is where _WIN32 defined too. Just use the
default macro instead of a reinvented one.
See thread "Replacing LLVM_ON_WIN32 with just _WIN32" on llvm-dev and cfe-dev.
No intended behavior change.
llvm-svn: 331069
The getVirtualFile method would create entries for e.g. libclang's
CXUnsavedFile but not mark them as valid. The effect is that a lookup
through getFile where the file name is not exactly matching the virtual
file (e.g. through mixing slashes and backslashes on Windows) would
result in a normal file "lookup", and re-using the file entry found
by using the UniqueID, and overwrite the file entry fields. Because the
lookup involves opening the file, and moving it into the file entry, the
file is now open. The SourceManager keys its buffers on the UniqueID
(which is still the same), so it will find an already loaded buffer.
Because only the loading a buffer from disk will close the file, the
FileEntry will hold on to an open file for as long as the FileManager
is around. As the FileManager will only get destroyed at a reparse,
you can't safe to the "leaked" and locked file on Windows.
llvm-svn: 298905
Differential Revision: http://reviews.llvm.org/D19843
Corresponding LLVM change: http://reviews.llvm.org/D19842
Re-commit after addressing issues with of generating too many warnings for Windows and asan test failures.
Patch by Eric Niebler
llvm-svn: 272562
addAncestorsAsVirtualDirs("<stdin>") quickly returns without doing work
because "<stdin>" has no parent_path. This violates the expectation
that a subsequent call to getDirectoryFromFile("<stdin>") would succeed.
Instead, it fails because it uses the "." if the file has no path
component.
Fix this by keeping the behavior between addAncestorsAsVirtualDirs and
getDirectoryFromFile symmetric.
llvm-svn: 266089
than reusing the "overridden buffer" mechanism. This will allow us to make
embedded files and overridden files behave differently in future.
llvm-svn: 254121
Summary: It breaks the build for the ASTMatchers
Subscribers: klimek, cfe-commits
Differential Revision: http://reviews.llvm.org/D13893
llvm-svn: 250827
Jan Korous