benlangmuir added inline comments.
================
Comment at: clang/unittests/Basic/FileEntryTest.cpp:47-52
FileEntryRef addFile(StringRef Name) {
- FEs.push_back(std::make_unique<FileEntry>());
+ const FileEntry *FE =
+ FM.getVirtualFile(std::to_string(UniqueNameCounter++), 0, 0);
return FileEntryRef(
- *Files.insert({Name, FileEntryRef::MapValue(*FEs.back().get(), DR)})
- .first);
+ *Files.try_emplace(Name, FileEntryRef::MapValue(*FE, DR)).first);
}
----------------
dexonsmith wrote:
> I don't love this from a layering perspective. FileEntryTest is testing the
> low-level pieces that are used inside FileManager... using FileManager to
> generate the FileEntries is awkward.
>
> Maybe it'd be okay if `FileManager::getVirtualFile()` were trivial, but it's
> not.
>
Could we add `friend class FileEntryTest` to `FileEntry` to get access to the
private constructor and go back to the original approach? I see some other
places in llvm that use that pattern.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D123197/new/
https://reviews.llvm.org/D123197
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
