[PATCH] D123197: Remove a few effectively-unused FileEntry APIs. NFC

[Ben Langmuir via Phabricator via cfe-commits]

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