[clang][HeaderSearch] Make sure there are no backslashes in suggestedPath

[sammccall]

This looks like a bug rather than a feature of HeaderSearch, shouldn't the slash conversion go there?

(It seems unlikely that diagnostics should suggest one thing but we insert another)

[kadircet]

I had just looked at the method name and thought it was the right place( suggestPathToFileForDiagnostics since it says "path to file".)

But looking at the comments,

/// Suggest a path by which the specified file could be found, for
/// use in diagnostics to suggest a #include.

HeaderSearch API itself seems like a better place, I suppose the confusion comes from the fact that paths and the include-directives are
the same on unix-like systems.

Updating the patch

[kadircet]

• Fix bug in HeaderSearch instead of clangd

[sammccall]

Thanks!
Can you document this in HeaderSearch.h?

[kadircet]

• Update documentation

[sammccall]

clang/include/clang/Lex/HeaderSearch.h

710 ↗

(On Diff #196392)

I don't think this is specific enough - on windows, backslashes are valid (or at least accepted by clang).
I'd suggest specifically mentioning forward slashes.

[kadircet]

• Update comments to focus on forward slashes

cfe/trunk/include/clang/Lex/HeaderSearch.h

search_dir_iterator system_dir_begin() const {
     return SearchDirs.begin() + SystemDirIdx;
   }
 
   search_dir_iterator system_dir_end() const { return SearchDirs.end(); }
 
   /// Retrieve a uniqued framework name.
   StringRef getUniqueFrameworkName(StringRef Framework);
 
-  /// Suggest a path by which the specified file could be found, for
-  /// use in diagnostics to suggest a #include.
+  /// Suggest a path by which the specified file could be found, for use in
+  /// diagnostics to suggest a #include. Returned path will only contain forward
+  /// slashes as separators.
   ///
   /// \param IsSystem If non-null, filled in to indicate whether the suggested
   ///        path is relative to a system header directory.
   std::string suggestPathToFileForDiagnostics(const FileEntry *File,
                                               bool *IsSystem = nullptr);
 
-  /// Suggest a path by which the specified file could be found, for
-  /// use in diagnostics to suggest a #include.
+  /// Suggest a path by which the specified file could be found, for use in
+  /// diagnostics to suggest a #include. Returned path will only contain forward
+  /// slashes as separators.
   ///
   /// \param WorkingDir If non-empty, this will be prepended to search directory
   /// paths that are relative.
   std::string suggestPathToFileForDiagnostics(llvm::StringRef File,
                                               llvm::StringRef WorkingDir,
                                               bool *IsSystem = nullptr);
 
   void PrintStats();

cfe/trunk/lib/Lex/HeaderSearch.cpp

for (auto NI = path::begin(File), NE = path::end(File),
 
       if (*NI != *DI)
         break;
     }
   }
 
   if (IsSystem)
     *IsSystem = BestPrefixLength ? BestSearchDir >= SystemDirIdx : false;
-  return File.drop_front(BestPrefixLength);
+  return path::convert_to_slash(File.drop_front(BestPrefixLength));
 }

cfe/trunk/unittests/Lex/HeaderSearchTest.cpp

EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/x/y/./z",
                                                    /*WorkingDir=*/""),
             "z");
   addSearchDir("a/.././c/");
   EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/m/n/./c/z",
                                                    /*WorkingDir=*/"/m/n/"),
             "z");
 }
 
+#ifdef _WIN32
+TEST_F(HeaderSearchTest, BackSlash) {
+  addSearchDir("C:\\x\\y\\");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("C:\\x\\y\\z\\t",
+                                                   /*WorkingDir=*/""),
+            "z/t");
+}
+#endif
+
 } // namespace
 } // namespace clang