[PATCH] D123144: FileManager: std::map => BumpPtrAllocator + DenseMap of pointers

Duncan P. N. Exon Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 6 10:53:59 PDT 2022

dexonsmith added inline comments.

Comment at: clang/lib/Basic/FileManager.cpp:318
   FileEntryRef ReturnedRef(*NamedFileEnt);
-  if (UFE.isValid()) { // Already have an entry with this inode, return it.
+  if (ReusingEntry) { // Already have an entry with this inode, return it.
sammccall wrote:
> dexonsmith wrote:
> > I'm trying to figure out why `ReusingEntry` was necessary. It looks to me like you could just call `UFE->isValid()` here and avoid introducing that `bool`... or am I missing something?
> You're right, but I think we can get rid of isValid, and plan to send a patch.
> A FileEntry handed out by FileManager always has isValid = true. It's not possible to create a useful FileEntry outside FileManager (and this is never done outside FileManagerTest). So the concept of validity isn't needed.
> (IMO the main cost of having the isValid flag is conceptual complexity - lots of places check it, which means people must be thinking about how to handle this case)
Sure, SGTM.

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list