[PATCH] D35092: Use native path syntax when writing PDB module name.

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: lld/COFF/PDB.cpp:248-249
     SmallString<128> Path = InArchive ? File->ParentName : File->getName();
+    sys::path::native(Path);
     sys::fs::make_absolute(Path);
     StringRef Name = InArchive ? File->getName() : StringRef(Path);
----------------
rnk wrote:
> We should explicitly pass sys::fs::style::windows to get the same behavior on Linux, right? Otherwise the test won't pass there.
I actually think we should use native path syntax.  We're already in uncharted waters dealing with cross-compilation because we'll be the first ones that have ever tried to do it, but since semantically the PDB file is supposed to contain "absolute path to the module on the file system where it was compiled", it seems like we need it to be a real path.  A windows tool isn't going to understand a path to a different machine anyway (even if it uses windows path syntax), so we might as well write it in a way that at least *some* system can make sense of the path.  We can fix the test by using a regex like `{{.*pdb-diff.obj}}


================
Comment at: lld/test/COFF/pdb-diff.test:175
 CHECK-NEXT:   |----------------------------------------+---|
-CHECK-NEXT:   |Module "D:\src\llvm-mono\lld\test\COFF/Inputs/pdb-diff.obj"|
+CHECK-NEXT:   |Module "D:\src\llvm-mono\lld\test\coff\Inputs\pdb-diff.obj"|
+CHECK-NEXT:   |----------------------------------------+---|
----------------
rnk wrote:
> This absolute path won't be the same on other people's systems.
Fixed offline.


https://reviews.llvm.org/D35092