[PATCH] D148213: [clangd] Use FileEntryRef for canonicalizing filepaths.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

we've got one more reference to `getCanonicalPath` in `clang-tools-extra/clangd/IncludeCleaner.cpp:320`, i guess the best way is just calling `getLastRef` on the FileEntry*



================
Comment at: clang-tools-extra/clangd/SourceCode.cpp:518
                                             const SourceManager &SourceMgr) {
   if (!F)
     return std::nullopt;
----------------
we don't need this check anymore


================
Comment at: clang-tools-extra/clangd/SourceCode.h:166
 /// possible.
-std::optional<std::string> getCanonicalPath(const FileEntry *F,
+std::optional<std::string> getCanonicalPath(const FileEntryRef &F,
                                             const SourceManager &SourceMgr);
----------------
just `FileEntryRef`, it's a cheap value type that provides an immutable view into underlying `FileEntry`


================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:208
   // We attempt to make the path absolute first.
-  const std::string &toURI(const FileEntry *FE) {
+  const std::string &toURI(const FileEntryRef &FE) {
     auto R = CacheFEToURI.try_emplace(FE);
----------------
again just `FileEntryRef`


================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:395
     auto &HS = PP->getHeaderSearchInfo();
-    if (const auto *HFI = HS.getExistingFileInfo(FE, /*WantExternal*/ false))
+    if (const auto *HFI = HS.getExistingFileInfo(&FE->getFileEntry(),
+                                                 /*WantExternal*/ false))
----------------
`FileEntryRef` should be implicitly convertable to `FileEntry*` (so just `*FE` should be enough?)


================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:399
+        if (auto Spelling = getFrameworkHeaderIncludeSpelling(
+                &FE->getFileEntry(), HFI->Framework, HS))
           return *Spelling;
----------------
same as above


================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:402
 
-    if (!tooling::isSelfContainedHeader(FE, PP->getSourceManager(),
+    if (!tooling::isSelfContainedHeader(&FE->getFileEntry(),
+                                        PP->getSourceManager(),
----------------
same as above


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148213