Hi Nico, I didn't think it necessary as the change doesn't introduce any interaction with filesystem - it's just copying a string.
Do you mean it causes a performance regression? Thanks. Jan > On Feb 15, 2019, at 6:15 AM, Nico Weber <tha...@chromium.org> wrote: > > Did you do any performance testing to check if this slows down clang? > > On Thu, Feb 14, 2019 at 6:02 PM Jan Korous via cfe-commits > <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: > Author: jkorous > Date: Thu Feb 14 15:02:35 2019 > New Revision: 354075 > > URL: http://llvm.org/viewvc/llvm-project?rev=354075&view=rev > <http://llvm.org/viewvc/llvm-project?rev=354075&view=rev> > Log: > [clang][FileManager] fillRealPathName even if we aren't opening the file > > The pathname wasn't previously filled when the getFile() method was called > with openFile = false. > We are caching FileEntry-s in ParsedAST::Includes in clangd and this caused > the problem. > > This fixes an internal test failure in clangd - ClangdTests.GoToInclude.All > > rdar://47536127 > > Differential Revision: https://reviews.llvm.org/D58213 > <https://reviews.llvm.org/D58213> > > Modified: > cfe/trunk/lib/Basic/FileManager.cpp > cfe/trunk/unittests/Basic/FileManagerTest.cpp > > Modified: cfe/trunk/lib/Basic/FileManager.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=354075&r1=354074&r2=354075&view=diff > > <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=354075&r1=354074&r2=354075&view=diff> > ============================================================================== > --- cfe/trunk/lib/Basic/FileManager.cpp (original) > +++ cfe/trunk/lib/Basic/FileManager.cpp Thu Feb 14 15:02:35 2019 > @@ -267,6 +267,9 @@ const FileEntry *FileManager::getFile(St > if (UFE.File) { > if (auto PathName = UFE.File->getName()) > fillRealPathName(&UFE, *PathName); > + } else if (!openFile) { > + // We should still fill the path even if we aren't opening the file. > + fillRealPathName(&UFE, InterndFileName); > } > return &UFE; > } > > Modified: cfe/trunk/unittests/Basic/FileManagerTest.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/FileManagerTest.cpp?rev=354075&r1=354074&r2=354075&view=diff > > <http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/FileManagerTest.cpp?rev=354075&r1=354074&r2=354075&view=diff> > ============================================================================== > --- cfe/trunk/unittests/Basic/FileManagerTest.cpp (original) > +++ cfe/trunk/unittests/Basic/FileManagerTest.cpp Thu Feb 14 15:02:35 2019 > @@ -346,4 +346,18 @@ TEST_F(FileManagerTest, getVirtualFileFi > EXPECT_EQ(file->tryGetRealPathName(), ExpectedResult); > } > > +TEST_F(FileManagerTest, getFileDontOpenRealPath) { > + auto statCache = llvm::make_unique<FakeStatCache>(); > + statCache->InjectDirectory("/tmp/abc", 42); > + SmallString<64> Path("/tmp/abc/foo.cpp"); > + statCache->InjectFile(Path.str().str().c_str(), 43); > + manager.setStatCache(std::move(statCache)); > + > + const FileEntry *file = manager.getFile(Path, /*openFile=*/false); > + > + ASSERT_TRUE(file != nullptr); > + > + ASSERT_EQ(file->tryGetRealPathName(), Path); > +} > + > } // anonymous namespace > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > <https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
