cameron314 added inline comments.
================
Comment at: lib/Basic/FileManager.cpp:304-307
@@ -303,1 +303,6 @@
+ if (UFE.isVirtual()) {
+ UFE.Name = InterndFileName;
+ return &UFE;
+ }
+
----------------
rsmith wrote:
> It looks like this is unreachable: `IsVirtual` is only ever `true` when
> `IsValid` is also `true`, and we don't reach this line if `UFE.isValid()`. As
> this is the only consumer of `FileEntry::IsValid`, it looks like it does
> nothing. Am I missing something?
Hmm, interesting, good catch. Back when I made this change UFE.IsValid was
//not// set to true in the fall-through after this if, which is why I had to
introduce `IsVirtual` in the first place.
But looking at the history on this file, it seems it //was// set to true since
the very beginning. I think we'd removed it in our local fork at one point to
work around a different bug, but it's back to normal in ours as well. So I
guess this part of the patch is unnecessary now. Yay! I'll remove it, it wasn't
very pretty anyway.
================
Comment at: lib/Frontend/ASTUnit.cpp:1402-1406
@@ +1401,7 @@
+
+ vfs::Status Status;
+ if (FileMgr->getNoncachedStatValue(RB.first, Status)) {
+ AnyFileChanged = true;
+ break;
+ }
+
----------------
rsmith wrote:
> If there are multiple file names with the same inode, `RB.first` is an
> arbitrarily-chosen one of those names, and stat'ing it isn't sufficient to
> check that none of the other names for the file have changed. Maybe we need
> to store a list of filenames used for each `UniqueID`?
I'm not sure I understand -- if the file changed on disk, why would it matter
which name we're accessing it by?
http://reviews.llvm.org/D20137
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
