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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
