Document 'use-external-name' and the various bits of logic that make it
work, to avoid others having to repeat the archival work (given that I
added getFileRefReturnsCorrectNameForDifferentStatPath to
FileManagerTest, seems possible I understood this once before!).
- b59cf679e8 added 'use-external-name' to
RedirectingFileSystem. This causes `stat`s to return the external
name for a redirected file instead of the name it was accessed by,
leaking it through the VFS.
- d066d4c849 propagated the external name
further through clang::FileManager.
- 4dc5573acc, which added
clang::FileEntryRef to clang::FileManager, has complicated concession
to account for this as well (since refactored a bit).
The goal of 'use-external-name' is to enable Clang to report "real" file
paths to users (via diagnostics) and to external tools (such as
debuggers reading debug info and build systems reading `.d` files).
I've added FIXMEs to look at other channels for communicating the
external names, since the current implementation adds complexity to
FileManager and exposes an inconsistent interface to clients.
Besides that, the FileManager logic appears to be kicking in outside of
'use-external-name'. Seems that *some* vfs::FileSystem implementations
canonicalize some paths returned by `stat` in *some* cases (the bug
isn't fully understood yet). Volodymyr Sapsai is investigating, this at
least better documents what *is* understood.
C++23 will make these conversions ambiguous - so fix them to make the
codebase forward-compatible with C++23 (& a follow-up change I've made
will make this ambiguous/invalid even in <C++23 so we don't regress
this & it generally improves the code anyway)
This is mostly a mechanical change, but a testcase that contains
parts of the StringRef class (clang/test/Analysis/llvm-conventions.cpp)
isn't touched.
After https://reviews.llvm.org/D90484 libclang is unable to read a serialized diagnostic file
which contains a diagnostic which came from a file with an empty filename. The reason being is
that the serialized diagnostic reader is creating a virtual file for the "" filename, which now
fails after the changes in https://reviews.llvm.org/D90484. This patch restores the previous
behavior in getVirtualFileRef by allowing it to construct a file entry ref with an empty name by
pretending its name is "." so that the directory entry can be created.
Differential Revision: https://reviews.llvm.org/D100428
Add support for stdin to SourceManager and FileManager. Adds
FileManager::getSTDIN, which adds a FileEntryRef for `<stdin>` and reads
the MemoryBuffer, which is stored as `FileEntry::Content`.
Eventually the other buffers in `ContentCache` will sink to here as well
-- we probably usually want to load/save a MemoryBuffer eagerly -- but
it's happening early for stdin to get rid of
CompilerInstance::InitializeSourceManager's final call to
`SourceManager::overrideFileContents`.
clang/test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.export/p1.cpp
relies on building a module from stdin; supporting that requires setting
ContentCache::BufferOverridden.
Differential Revision: https://reviews.llvm.org/D93148
Handle named pipes natively in SourceManager and FileManager, removing a
call to `SourceManager::overrideFileContents` in
`CompilerInstance::InitializeSourceManager` (removing a blocker for
sinking the content cache to FileManager (which will incidently sink
this new named pipe logic with it)).
SourceManager usually checks if the file entry's size matches the
eventually loaded buffer, but that's now skipped for named pipes since
the `stat` won't reflect the full size. Since we can't trust
`ContentsEntry->getSize()`, we also need shift the check for files that
are too large until after the buffer is loaded... and load the buffer
immediately in `createFileID` so that no client gets a bad value from
`ContentCache::getSize`. `FileManager::getBufferForFile` also needs to
treat these files as volatile when loading the buffer.
Native support in SourceManager / FileManager means that named pipes can
also be `#include`d, and clang/test/Misc/dev-fd-fs.c was expanded to
check for that.
This is a new version of 3b18a594c7, which
was reverted in b346322019 since it was
missing the `SourceManager` changes.
Differential Revision: https://reviews.llvm.org/D92531
This reverts commit 3b18a594c7, since
apparently this doesn't work everywhere. E.g.,
clang-x86_64-debian-fast/3889
(http://lab.llvm.org:8011/#/builders/109/builds/3889) gives me:
```
+ : 'RUN: at line 8'
+ /b/1/clang-x86_64-debian-fast/llvm.obj/bin/clang -x c /dev/fd/0 -E
+ cat /b/1/clang-x86_64-debian-fast/llvm.src/clang/test/Misc/dev-fd-fs.c
fatal error: file '/dev/fd/0' modified since it was first processed
1 error generated.
```
Remove compilicated logic from CompilerInstance::InitializeSourceManager
to deal with named pipes, updating FileManager::getBufferForFile to
handle it in a more straightforward way. The existing test at
clang/test/Misc/dev-fd-fs.c covers the new behaviour (just like it did
the old behaviour).
Differential Revision: https://reviews.llvm.org/D90733
Add `FileEntryRef::getDir`, which returns a `DirectoryEntryRef`. This
includes a few changes:
- Customize `OptionalStorage` so that `Optional<DirectoryEntryRef>` is
pointer-sized (like the change made to `Optional<FileEntryRef>`).
Factored out a common class, `FileMgr::MapEntryOptionalStorage`, to
reduce the code duplication.
- Store an `Optional<DirectoryEntryRef>` in `FileEntryRef::MapValue`.
This is set if and only if `MapValue` has a real `FileEntry`.
- Change `FileManager::getFileRef` and `getVirtualFileRef` to use
`getDirectoryRef` and store it in the `StringMap` for `FileEntryRef`.
Differential Revision: https://reviews.llvm.org/D90484
Change the `InputFile` class to store `Optional<FileEntryRef>` instead
of `FileEntry*`. This paged in a few API changes:
- Added `FileManager::getVirtualFileRef`, and converted `getVirtualFile`
to a wrapper of it.
- Updated `SourceManager::bypassFileContentsOverride` to take
`FileEntryRef` and return `Optional<FileEntryRef>`
(`ASTReader::getInputFile` is the only caller).
Differential Revision: https://reviews.llvm.org/D90053
Shrink `FileEntryRef` to the size of a pointer, by having it directly
reference the `StringMapEntry` the same way that `DirectoryEntryRef`
does. This makes `FileEntryRef::FileEntryRef` private as a side effect
(`FileManager` is a friend!).
There are two helper types added within `FileEntryRef`:
- `FileEntryRef::MapValue` is the type stored in
`FileManager::SeenFileEntries`. It's a replacement for
`SeenFileEntryOrRedirect`, where the second pointer type has been
changed from `StringRef*` to `MapEntry*` (see next bullet).
- `FileEntryRef::MapEntry` is the instantiation of `StringMapEntry<>`
where `MapValue` is stored. This is what `FileEntryRef` has a pointer
to, in order to grab the name in addition to the value.
Differential Revision: https://reviews.llvm.org/D89488
This is needed to fix the reason
0a2be46cfd (Modules: Invalidate out-of-date PCMs as they're
discovered) and 5b44a4b07fc1d ([modules] Do not cache invalid state for
modules that we attempted to load.) were reverted.
These patches changed Clang to use `isVolatile` when loading modules.
This had the side effect of not using mmap when loading modules, and
thus greatly increased memory usage.
The reason it wasn't using mmap is because `MemoryBuffer` plays some
games with file size when you request null termination, and it has to
disable these when `isVolatile` is set as the size may change by the
time it's mmapped. Clang by default passes
`RequiresNullTerminator = true`, and `shouldUseMmap` ignored if
`RequiresNullTerminator` was even requested.
This patch adds `RequiresNullTerminator` to the `FileManager` interface
so Clang can use it when loading modules, and changes `shouldUseMmap` to
only take volatility into account if `RequiresNullTerminator` is true.
This is fine as both `mmap` and a `read` loop are vulnerable to
modifying the file while reading, but are immune to the rename Clang
does when replacing a module file.
Differential Revision: https://reviews.llvm.org/D77772
This is how it should've been and brings it more in line with
std::string_view. There should be no functional change here.
This is mostly mechanical from a custom clang-tidy check, with a lot of
manual fixups. It uncovers a lot of minor inefficiencies.
This doesn't actually modify StringRef yet, I'll do that in a follow-up.
In the current implementation of clang the canonicalization of paths in
diagnostic messages (when using -fdiagnostics-absolute-paths) only works
if the symbolic link is in the directory part of the filename, not if
the file itself is a symbolic link to another file.
This patch adds support to canonicalize the complete path including the
file.
Reviewers: rsmith, hans, rnk, ikudrin
Reviewed By: rnk
Differential Revision: https://reviews.llvm.org/D70527
accessed name to the directory entry
This commit introduces a parallel API that returns a DirectoryEntryRef
to the FileManager, similar to the parallel FileEntryRef API. All
uses will have to be update in follow-up patches. The immediate use of the new API in this
patch fixes the issue where a file manager was reused in clang-scan-deps,
but reported an different file path whenever a framework lookup was done through a symlink.
Differential Revision: https://reviews.llvm.org/D67026
llvm-svn: 370562
If contents of a file that is part of a PCM are overridden when reading
it, but weren't overridden when the PCM was being built, the ASTReader
will emit an error. Now it creates a separate FileEntry for recovery,
bypassing the overridden content instead of discarding it. The
pre-existing testcase clang/test/PCH/remap-file-from-pch.cpp confirms
that the new recovery method works correctly.
This resolves a long-standing FIXME to avoid hypothetically invalidating
another precompiled module that's already using the overridden contents.
This also removes ContentCache-related API that would be unsafe to use
across `CompilerInstance`s in an implicit modules build. This helps to
unblock us sinking it from SourceManager into FileManager in the future,
which would allow us to delete `InMemoryModuleCache`.
https://reviews.llvm.org/D66710
llvm-svn: 370546
`FileManager::getFileRef` is a modern API which we expect to convert to
over time. We should modernize the error handling as well, using
`llvm::Expected` instead of `llvm::ErrorOr`, to help clients that care
about errors to ensure nothing is missed.
However, not all clients care. I've also added another path for those
that don't:
- `FileEntryRef` is now copy- and move-assignable (using a pointer
instead of a reference).
- `FileManager::getOptionalFileRef` returns an `llvm::Optional` instead
of `llvm::Expected`.
- Added an `llvm::expectedToOptional` utility in case this is useful
elsewhere.
https://reviews.llvm.org/D66705
llvm-svn: 369943
when the FileManager is reused across invocations
This commit introduces a parallel API to FileManager's getFile: getFileEntryRef, which returns
a reference to the FileEntry, and the name that was used to access the file. In the case of
a VFS with 'use-external-names', the FileEntyRef contains the external name of the file,
not the filename that was used to access it.
The new API is adopted only in the HeaderSearch and Preprocessor for include file lookup, so that the
accessed path can be propagated to SourceManager's FileInfo. SourceManager's FileInfo now can report this accessed path, using
the new getName method. This API is then adopted in the dependency collector, which now correctly reports dependencies when a file
is included both using a symlink and a real path in the case when the FileManager is reused across multiple Preprocessor invocations.
Note that this patch does not fix all dependency collector issues, as the same problem is still present in other cases when dependencies
are obtained using FileSkipped, InclusionDirective, and HasInclude. This will be fixed in follow-up commits.
Differential Revision: https://reviews.llvm.org/D65907
llvm-svn: 369680
Now that we've moved to C++14, we no longer need the llvm::make_unique
implementation from STLExtras.h. This patch is a mechanical replacement
of (hopefully) all the llvm::make_unique instances across the monorepo.
Differential revision: https://reviews.llvm.org/D66259
llvm-svn: 368942
Previously, the FileManager would use NULL returns to signify whether a file existed, but that doesn’t cover permissions issues or anything else that might occur while trying to stat or read a file. Instead, convert getFile and getDirectory into returning llvm::ErrorOr
Signed-off-by: Harlan Haskins <harlan@apple.com>
llvm-svn: 367615
Summary:
Previously, we would return true/false signifying if the cache/lookup
succeeded or failed. Instead, provide clients with the underlying error
that was thrown while attempting to look up in the cache.
Since clang::FileManager doesn't make use of this information, it discards the
error that's received and casts away to bool.
This change is NFC.
Reviewers: benlangmuir, arphaman
Subscribers: dexonsmith, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60735
llvm-svn: 358509
Summary:
FileData was only ever used as a container for the values in
llvm::vfs::Status, so they might as well be consolidated.
The `InPCH` member was also always set to false, and unused.
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D58924
llvm-svn: 355368
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
Duncan P. N. Exon Smith