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

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith planned changes to this revision.
dexonsmith added inline comments.


================
Comment at: clang/lib/Basic/FileManager.cpp:219
+      return FileEntryRef(*SeenFileInsertResult.first);
+    return FileEntryRef(*Value.V.get<const FileEntryRef::MapEntry *>());
   }
----------------
arphaman wrote:
> 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?
I augmented the test for getFileRef to check for multi-level redirections. It asserts both before and after this patch. Here is a partial backtrace from before:
```
Assertion failed: (is<T>() && "Invalid accessor called"), function get, file llvm/include/llvm/ADT/PointerUnion.h, line 188.
[...]
8  BasicTests               0x0000000103fb5ce9 clang::FileEntry* llvm::PointerUnion<clang::FileEntry*, llvm::StringRef const*>::get<clang::FileEntry*>() const + 105
9  BasicTests               0x0000000103fb4f80 clang::FileManager::getFileRef(llvm::StringRef, bool, bool) + 2240
10 BasicTests               0x0000000103d27d57 (anonymous namespace)::FileManagerTest_getFileRefReturnsCorrectNameForDifferentStatPath_Test::TestBody() + 711
```
Can you confirm this is the right test to add, or did you mean something different?


================
Comment at: clang/unittests/Basic/FileManagerTest.cpp:294
   auto F1Alias2 = manager.getFileRef("dir/f1-alias.cpp");
+  auto F1AliasAlias = manager.getFileRef("dir/f1-alias-alias.cpp");
   ASSERT_FALSE(!F1);
----------------
@arphaman, this is the line the test asserts on (with or without my patch).


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

https://reviews.llvm.org/D89488