================
@@ -82,26 +82,39 @@ std::optional<Path> getCorrespondingHeaderOrSource(PathRef 
OriginalFile,
       Request.IDs.insert(ID);
   }
   llvm::StringMap<int> Candidates; // Target path => score.
-  auto AwardTarget = [&](const char *TargetURI) {
-    if (auto TargetPath = URI::resolve(TargetURI, OriginalFile)) {
-      if (!pathEqual(*TargetPath, OriginalFile)) // exclude the original file.
-        ++Candidates[*TargetPath];
-    } else {
-      elog("Failed to resolve URI {0}: {1}", TargetURI, 
TargetPath.takeError());
-    }
-  };
-  // If we switch from a header, we are looking for the implementation
-  // file, so we use the definition loc; otherwise we look for the header file,
-  // we use the decl loc;
+  // When in the implementation file, we always search for the header file,
+  // using the decl loc. When we are in a header, this usually implies 
searching
+  // for implementation, for which we use the definition loc. For templates, we
+  // can have separate implementation headers, which behave as an 
implementation
+  // file. As such, we always have to add the decl loc and conditionally
+  // definition loc.
   //
   // For each symbol in the original file, we get its target location (decl or
   // def) from the index, then award that target file.
-  const bool IsHeader = isHeaderFile(OriginalFile, AST.getLangOpts());
+#ifdef CLANGD_PATH_CASE_INSENSITIVE
+  auto pathEqual = [](llvm::StringRef l, llvm::StringRef r) {
+    return l.equals_insensitive(r);
+  };
+#else
+  auto pathEqual = [](llvm::StringRef l, llvm::StringRef r) { return l == r; };
+#endif
   Index->lookup(Request, [&](const Symbol &Sym) {
-    if (IsHeader)
-      AwardTarget(Sym.Definition.FileURI);
-    else
-      AwardTarget(Sym.CanonicalDeclaration.FileURI);
+    if (llvm::StringRef{Sym.Definition.FileURI}.empty() ||
----------------
JVApen wrote:

I did that originally and stepped away from that. It was based on user 
feedback. If I remember well, we had some forward declares of classes in our 
headers. These where also picked up and the switch went to the header linked to 
the forward declared class.

This triggered when the impl file only had a single method implemented. That 
caused the header of the forward declare and the correct header to both have a 
count of 1, in which case it picked consistently the wrong one. As soon as you 
had 2 methods in the impl, it worked as expected.

So instead of figuring out if a visited symbol was relevant based on its usage, 
the check if either definition or declaration is in the current file provided a 
more solid solution in picking the right one.

In the previous implementation, this wasn't a problem given that in headers it 
only searched for the definition.

https://github.com/llvm/llvm-project/pull/158461
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to