Reland "[Lex] Fix suggested spelling of /usr/bin/../include/foo"

[sammccall]

Summary

This reverts commit 1dc0a1e5d220b83c1074204bd3afd54f3bac4270.

Failures were caused by unintentional conversion to native slashes by
remove_dots, so undo that: we always suggest posix slashes for includes.

This could potentially be a change in behavior on windows if people were
spelling headers with backslashes and headermaps contained backslashes,
but that's all underspecified and I don't think anyone uses headermaps
on windows.

[kadircet]

thanks!

clang/lib/Lex/HeaderSearch.cpp

1936

might be worth adding a comment like we can't pass posix as style to remove_dots, because it won't canonicalize "a\.\b.h"

clang/unittests/Lex/HeaderSearchTest.cpp

155

can you also add a new test that looks like:

addSearchDir("x/");
EXPECT(suggestForDiag("x\y\z.h"), "y/z.h");

as in theory that's the new behaviour we're adding.

[sammccall]

clang/unittests/Lex/HeaderSearchTest.cpp

155

That looks a lot like TEST_F(HeaderSearchTest, BackSlash) to me (relative vs absolute, but not relevant at least to this change)

clang/lib/Lex/HeaderSearch.cpp

std::string HeaderSearch::suggestPathToFileForDiagnostics(
   return suggestPathToFileForDiagnostics(File->getName(), /*WorkingDir=*/"",
                                          MainFile, IsSystem);
 }
 
 std::string HeaderSearch::suggestPathToFileForDiagnostics(
     llvm::StringRef File, llvm::StringRef WorkingDir, llvm::StringRef MainFile,
     bool *IsSystem) {
   using namespace llvm::sys;
   
+  llvm::SmallString<32> FilePath = File;
+  // remove_dots switches to backslashes on windows as a side-effect!
+  // We always want to suggest forward slashes for includes.
+  // (not remove_dots(..., posix) as that misparses windows paths).
+  path::remove_dots(FilePath, /*remove_dot_dot=*/true);
+  path::native(FilePath, path::Style::posix);
+  File = FilePath;
+
   unsigned BestPrefixLength = 0;
   // Checks whether `Dir` is a strict path prefix of `File`. If so and that's
   // the longest prefix we've seen so for it, returns true and updates the
   // `BestPrefixLength` accordingly.
-  auto CheckDir = [&](llvm::StringRef Dir) -> bool {
-    llvm::SmallString<32> DirPath(Dir.begin(), Dir.end());
+  auto CheckDir = [&](llvm::SmallString<32> Dir) -> bool {
     if (!WorkingDir.empty() && !path::is_absolute(Dir))
-      fs::make_absolute(WorkingDir, DirPath);
-    path::remove_dots(DirPath, /*remove_dot_dot=*/true);
-    Dir = DirPath;
+      fs::make_absolute(WorkingDir, Dir);
+    path::remove_dots(Dir, /*remove_dot_dot=*/true);
     for (auto NI = path::begin(File), NE = path::end(File),
               DI = path::begin(Dir), DE = path::end(Dir);
-         /*termination condition in loop*/; ++NI, ++DI) {
-      // '.' components in File are ignored.
-      while (NI != NE && *NI == ".")
-        ++NI;
-      if (NI == NE)
-        break;
-
-      // '.' components in Dir are ignored.
-      while (DI != DE && *DI == ".")
-        ++DI;
+         NI != NE; ++NI, ++DI) {
       if (DI == DE) {
-        // Dir is a prefix of File, up to '.' components and choice of path
-        // separators.
+        // Dir is a prefix of File, up to choice of path separators.
         unsigned PrefixLength = NI - path::begin(File);
         if (PrefixLength > BestPrefixLength) {
           BestPrefixLength = PrefixLength;
           return true;
         }
         break;
       }
 

clang/unittests/Lex/HeaderSearchTest.cpp

 TEST_F(HeaderSearchTest, DotDotsWithAbsPath) {
   addSearchDir("/x/../y/");
   EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/y/z",
                                                    /*WorkingDir=*/"",
                                                    /*MainFile=*/""),
             "z");
 }
 
+TEST_F(HeaderSearchTest, BothDotDots) {
+  addSearchDir("/x/../y/");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/x/../y/z",
+                                                   /*WorkingDir=*/"",
+                                                   /*MainFile=*/""),
+            "z");
+}
+
 TEST_F(HeaderSearchTest, IncludeFromSameDirectory) {
   EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/y/z/t.h",
                                                    /*WorkingDir=*/"",
                                                    /*MainFile=*/"/y/a.cc"),
             "z/t.h");
 
   addSearchDir("/");
   EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/y/z/t.h",