ioeric added inline comments.
================
Comment at: lib/Basic/SourceManager.cpp:1626
+ if (FileContentCache->OrigEntry == SourceFile) {
+ if (Callback(FileID::get(I)))
+ return true;
----------------
Should we check `FileID::get(I)` is valid?
================
Comment at: lib/Basic/SourceManager.cpp:1628
+ return true;
+ } else if (LookForFilesystemUpdates &&
+ (SourceFileName || (SourceFileName = llvm::sys::path::filename(
----------------
The conditions here are pretty hard to follow and reason about. Could we maybe
split them (some documentation would also help)?
================
Comment at: lib/Basic/SourceManager.cpp:1628
+ return true;
+ } else if (LookForFilesystemUpdates &&
+ (SourceFileName || (SourceFileName = llvm::sys::path::filename(
----------------
ioeric wrote:
> The conditions here are pretty hard to follow and reason about. Could we
> maybe split them (some documentation would also help)?
In the original version, file system updates are checked last (after modules).
Any reason to move it before modules?
Also, it seems that this code path could also be run when `FileID`above is
invalid? So I wonder whether `else if` is correct here.
https://reviews.llvm.org/D50926
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
