Do not resolve directory junctions for `-fdiagnostics-absolute-paths` on Windows.

[ikudrin]

Summary

This patch partially restores the behavior changed in D46942/rC332590

If the source file path contains directory junctions, and we resolve them when printing
diagnostic messages, these paths look independent for an IDE. For example, both Visual
Studio and Visual Studio Code open separate editors for such paths, which is not only
inconvenient but might even result in loss of the changes made in one of them.

[hans]

test/Frontend/absolute-paths-windows.test

5

This requires (a recent version of) Win 10 or later to run without running as Admin, right?

[ikudrin]

test/Frontend/absolute-paths-windows.test

5

As far as I know, creating a directory junction, in contrast to a symbolic link, does not require escalated privileges. But, honestly, I do not have any old system to validate that.

[ikudrin]

test/Frontend/absolute-paths-windows.test

5

We checked that on Windows 7. It does not require rights elevation, too.

[ikudrin]

Ping.

Please note, that, in contrary to POSIX OSes, on Windows a path like dir1\<junction point>\.. refers to dir1. That's why we do not need to ask the system for the fully expanded path and can do all calculations internally.

[hans]

I don't feel like I have enough background on what you're trying to do to review this. It sounds related to clangd issues maybe?

"If the source file path contains directory junctions, and we resolve them when printing diagnostic messages"

I didn't think Clang tried to resolve symlinks or otherwise canonicalize paths when refering to files in diagnostics in the first place?

[ikudrin]

• Made the patch affect only -fdiagnostics-absolute-paths option.

[hans]

lib/Frontend/TextDiagnostic.cpp

768

I think this requires a comment explaining why Windows is handled differently.

[ikudrin]

Added a comment explaining the differences in the implementations. I would really appreciate any corrections.

[hans]

Thanks! This looks good to me.

I'm still a bit nervous about "mklink" in the test, but let's land this and see if the bots can handle it.

[johan-boule]

Is this going to break symlinks on Windows?

[ikudrin]

I am not sure what you mean by "break". Can you share a reproducer?

lib/Frontend/TextDiagnostic.cpp

 }
 
 void TextDiagnostic::emitFilename(StringRef Filename, const SourceManager &SM) {
   SmallVector<char, 128> AbsoluteFilename;
   if (DiagOpts->AbsolutePath) {
     const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
         llvm::sys::path::parent_path(Filename));
     if (Dir) {
+      // We want to print a simplified absolute path, i. e. without "dots".
+      //
+      // The hardest part here are the paths like "<part1>/<link>/../<part2>".
+      // On Unix-like systems, we cannot just collapse "<link>/..", because
+      // paths are resolved sequentially, and, thereby, the path
+      // "<part1>/<part2>" may point to a different location. That is why
+      // we use FileManager::getCanonicalName(), which expands all indirections
+      // with llvm::sys::fs::real_path() and caches the result.
+      //
+      // On the other hand, it would be better to preserve as much of the
+      // original path as possible, because that helps a user to recognize it.
+      // real_path() expands all links, which sometimes too much. Luckily,
+      // on Windows we can just use llvm::sys::path::remove_dots(), because,
+      // on that system, both aforementioned paths point to the same place.
+#ifdef _WIN32
+      SmallString<4096> DirName = Dir->getName();
+      llvm::sys::fs::make_absolute(DirName);
+      llvm::sys::path::native(DirName);
+      llvm::sys::path::remove_dots(DirName, /* remove_dot_dot */ true);
+#else
       StringRef DirName = SM.getFileManager().getCanonicalName(Dir);
+#endif
       llvm::sys::path::append(AbsoluteFilename, DirName,
                               llvm::sys::path::filename(Filename));
       Filename = StringRef(AbsoluteFilename.data(), AbsoluteFilename.size());
     }
   }
 
   OS << Filename;
 }

test/Frontend/absolute-paths-windows.test

+// REQUIRES: system-windows
+// RUN: rm -rf %t.dir
+// RUN: mkdir -p %t.dir\real
+// RUN: cmd /c mklink /j %t.dir\junc %t.dir\real
+// RUN: echo "wrong code" > %t.dir\real\foo.cpp
+// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-absolute-paths %t.dir\junc\foo.cpp 2>&1 | FileCheck %s
+
+// CHECK-NOT: .dir\real\foo.cpp
+// CHECK: .dir\junc\foo.cpp

test/Frontend/lit.local.cfg

-config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl']
+config.suffixes = ['.c', '.cpp', '.m', '.mm', '.ll', '.cl', '.test']