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);
}
----------------
benlangmuir wrote:
> 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.
(just saw Sam made the same suggestion)
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
