jansvoboda11 added a comment.
The failing test was a fun one to debug. It didn't test what it intended to. It
wanted to check that header referenced from module map is found via a search
path, but the command line was constructed in such way that the header was
found relative to the includer. The test relied on the `FileManager` hackery
that mutates `FileEntry` objects. Now that we call
`SourceMgr.getFileEntryRefForID()`, we don't see effects of this hack. I ended
up only fixing the test, since removing the `FileManager` hack safely probably
requires us to remove all remaining `{File,Directory}Entry::getName()` usages.
LMK if you have more feedback.
================
Comment at: clang/lib/Frontend/FrontendAction.cpp:811
// Relative searches begin from CWD.
- const DirectoryEntry *Dir = nullptr;
- if (auto DirOrErr = CI.getFileManager().getDirectory("."))
- Dir = *DirOrErr;
- SmallVector<std::pair<const FileEntry *, const DirectoryEntry *>, 1> CWD;
- CWD.push_back({nullptr, Dir});
+ auto Dir = CI.getFileManager().getOptionalDirectoryRef(".");
+ SmallVector<std::pair<const FileEntry *, DirectoryEntryRef>, 1> CWD;
----------------
bnbarham wrote:
> Worth adding an assert here? It'd be nice to cleanup the CWD handling in
> FileManager at some point.
Assert that `Dir` is not empty? Seems overly defensive to me, what would be the
benefit of that? Catching if the working directory was yanked from underneath
Clang?
================
Comment at: clang/lib/Lex/PPDirectives.cpp:891
// stack, record the parent #includes.
- SmallVector<std::pair<const FileEntry *, const DirectoryEntry *>, 16>
- Includers;
+ SmallVector<std::pair<const FileEntry *, DirectoryEntryRef>, 16> Includers;
bool BuildSystemModule = false;
----------------
bnbarham wrote:
> Could we change this to a FileEntryRef as well, or would you prefer to do
> that later? It'll touch basically all the same places again (eg. each
> parameter above).
I'd prefer to do it separately for smaller diff and easier bisection. LMK if
you have strong preference for doing it in the same commit.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127663/new/
https://reviews.llvm.org/D127663
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
