sammccall added inline comments.
================
Comment at: clang/lib/Basic/SourceManager.cpp:1805
+ // macro args expanded in the main file.
+ if (Entry.getFile().Filename == "<built-in>" && FID == getMainFileID()) {
+ if (Entry.getFile().NumCreatedFIDs)
----------------
nit: reverse the order of this check to avoid the string compare?
You could also consider hoisting `bool IsMainFile` out of the loop, maybe the
optimizer can see this but I think it's probably a readablity win anyway.
================
Comment at: clang/lib/Basic/SourceManager.cpp:1806
+ if (Entry.getFile().Filename == "<built-in>" && FID == getMainFileID()) {
+ if (Entry.getFile().NumCreatedFIDs)
+ ID += Entry.getFile().NumCreatedFIDs - 1 /*because of next ++ID*/;
----------------
I don't love having this code duplicated.
What about:
```
SourceLocation IncludeLoc = ...
bool IncludedFromMain = isInFileID(IncludeLoc, FID) || (IsMainFile && Filename
= "<built-in>");
if (IncludedFromMain) {
// increment ID
} else if (IncludeLoc.isValid()) { // included, but not from this file
return
}
continue;
```
================
Comment at: clang/lib/Lex/PPLexerChange.cpp:420
+ // Predefines file doesn't have a valid include location.
+ CurPPLexer->FID == getPredefinesFileID())) {
// Notify SourceManager to record the number of FileIDs that were created
----------------
Can we ever get spurious equality here because both sides are 0?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78649/new/
https://reviews.llvm.org/D78649
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
