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

4

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

[ikudrin]

test/Frontend/absolute-paths-windows.test

4

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

4

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 ↗

(On Diff #200252)

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/Basic/FileManager.cpp

StringRef FileManager::getCanonicalName(const DirectoryEntry *Dir) {
   llvm::DenseMap<const DirectoryEntry *, llvm::StringRef>::iterator Known
     = CanonicalDirNames.find(Dir);
   if (Known != CanonicalDirNames.end())
     return Known->second;
 
   StringRef CanonicalName(Dir->getName());
 
   SmallString<4096> CanonicalNameBuf;
+#ifdef _WIN32
+  CanonicalNameBuf = CanonicalName;
+  llvm::sys::fs::make_absolute(CanonicalNameBuf);
+  llvm::sys::path::native(CanonicalNameBuf);
+  // We've run into needing to remove '..' here in the wild though, so
+  // remove it.
+  // On Windows, symlinks are significantly less prevalent, so removing
+  // '..' is pretty safe.
+  // Ideally we'd have an equivalent of `realpath` and could implement
+  // sys::fs::canonical across all the platforms.
+  llvm::sys::path::remove_dots(CanonicalNameBuf, /* remove_dot_dot */ true);
+  CanonicalName = StringRef(CanonicalNameBuf).copy(CanonicalNameStorage);
+#else
   if (!FS->getRealPath(Dir->getName(), CanonicalNameBuf))
     CanonicalName = StringRef(CanonicalNameBuf).copy(CanonicalNameStorage);
+#endif
 
   CanonicalDirNames.insert({Dir, CanonicalName});
   return CanonicalName;
 }
 
 void FileManager::PrintStats() const {
   llvm::errs() << "\n*** File Manager Stats:\n";
   llvm::errs() << UniqueRealFiles.size() << " real files found, "

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']