[PATCH] D103142: [clang][clangd] Use reverse header map lookup in suggestPathToFileForDiagnostics

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added subscribers: JDevlieghere, vsapsai.
bruno added inline comments.


================
Comment at: clang/lib/Lex/HeaderSearch.cpp:1851
 
-
-  return path::convert_to_slash(File.drop_front(BestPrefixLength));
+  // Try resolving resulting filaname via reverse search in header maps,
+  // key from header name is user prefered name for the include file.
----------------
-> `filename`


================
Comment at: clang/lib/Lex/HeaderSearch.cpp:1855
+  for (unsigned I = 0; I != SearchDirs.size(); ++I) {
+    if (SearchDirs[I].isHeaderMap()) {
+      StringRef SpelledFilename =
----------------
Can we save some dir scanning time by adding this logic to the previous loop? Shouldn't get hard to read if you early `continue` for each failed condition.


================
Comment at: clang/lib/Lex/HeaderSearch.cpp:1859
+      if (!SpelledFilename.empty()) {
+        Filename = SpelledFilename;
+        break;
----------------
I guess one of the rationale behind this change is that whatever is consuming your diagnostics is expecting the hmap key entry as an actionable path they could use.

I wonder whether other consumers of `suggestPathToFileForDiagnostics` expect an actual mapped value or just don't care. If the former, this might be better under some flag.

@dexonsmith @vsapsai @JDevlieghere @arphaman how does this relate with what you expect out of `suggestPathToFileForDiagnostics`? (If at all).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103142