[PATCH] D89488: FileManager: Shrink FileEntryRef to the size of a pointer

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.


================
Comment at: clang/lib/Basic/FileManager.cpp:217
     FileEntry *FE;
-    if (LLVM_LIKELY(FE = Value.dyn_cast<FileEntry *>()))
-      return FileEntryRef(SeenFileInsertResult.first->first(), *FE);
-    return getFileRef(*Value.get<const StringRef *>(), openFile, CacheFailure);
+    if (LLVM_LIKELY(FE = Value.V.dyn_cast<FileEntry *>()))
+      return FileEntryRef(*SeenFileInsertResult.first);
----------------
Can you use `isa` here instead of `dyn_cast`?


================
Comment at: clang/lib/Basic/FileManager.cpp:219
+      return FileEntryRef(*SeenFileInsertResult.first);
+    return FileEntryRef(*Value.V.get<const FileEntryRef::MapEntry *>());
   }
----------------
It looks like this accounts for one level of redirections, but the previous implementation could support multiple levels of redirections. Can multiple levels of redirections still be supported here too?


================
Comment at: clang/lib/Basic/FileManager.cpp:309
     // adopt FileEntryRef.
-    UFE.Name = InterndFileName;
-
-    return FileEntryRef(InterndFileName, UFE);
+    return *(UFE.LastRef = FileEntryRef(*NamedFileEnt));
   }
----------------
Can you rewrite the FIXME above, and also the assignment return makes it less readable. Maybe separate out the return onto the next line?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89488/new/

https://reviews.llvm.org/D89488