ilya-biryukov added inline comments.
================
Comment at: include/clang/Basic/FileManager.h:177
+ /// Fills the RealPathName entry in UFE by converting the given filename to
an
+ /// absolute path, returns true if FileName was modified.
+ bool fillRealPathName(FileEntry *UFE, llvm::StringRef FileName);
----------------
The FIXME inside the function suggests we might change the name or semantics of
RealPathName, so maybe simply refer to the field by name and avoid mentioning
the semantics of the function.
================
Comment at: unittests/Basic/FileManagerTest.cpp:237
ASSERT_TRUE(file->isValid());
// "real path name" reveals whether the file was actually opened.
EXPECT_EQ("", file->tryGetRealPathName());
----------------
This test seems to rely on the old behavior. Is there a way to test the file
wasn't open other than looking at RealPathName?
================
Comment at: unittests/Basic/FileManagerTest.cpp:238
// "real path name" reveals whether the file was actually opened.
EXPECT_EQ("", file->tryGetRealPathName());
----------------
Should we fill-in the RealPathName in that case as well?
I'm not sure what the semantics should be. WDYT?
================
Comment at: unittests/Basic/FileManagerTest.cpp:253
file = manager.getFile("/tmp/testv", /*OpenFile=*/true);
- EXPECT_EQ("", file->tryGetRealPathName());
}
----------------
We should find a way to test file was not "opened" (I think it actually does
stat the file now, right?)
================
Comment at: unittests/Basic/FileManagerTest.cpp:327
+
+ EXPECT_EQ(file1->tryGetRealPathName(), file2->tryGetRealPathName());
}
----------------
After a check `EXPECT_EQ(file1, file2)`, this is clearly true.
Maybe add a smaller test with a single call to `getVirtualFile()` and an
assertion that the real path is set?
Alternatively, add an assertion to this test right after the first call to
`getVirtualFile()`.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55054/new/
https://reviews.llvm.org/D55054
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
