[PATCH] D147256: [DebugInfo] Fix file path separator when targeting windows.

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Thanks for working on this!

The `-ffile-reproducible` flag name refers to making `#file` directives reproducible, but `LangOptions.UseTargetPathSeparator` sounds a lot broader :) I don't know what others think, but it would be nice to not have to introduce any more flags at least.



================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:544
     MainFileDir = std::string(MainFile->getDir().getName());
     if (!llvm::sys::path::is_absolute(MainFileName)) {
       llvm::SmallString<1024> MainFileDirSS(MainFileDir);
----------------
Do we want to fix absolute filenames too?
I can see arguments for and against:
- Using what the user provided makes sense
- Some build systems might use absolute paths for everything. But on the other hand such builds have larger determinism problems (including the full paths).
So the current decision probably makes sense.


================
Comment at: clang/test/CodeGen/debug-info-slash.c:5
+
+// WIN:   !DIFile(filename: "{{.*}}clang/test/CodeGen\\debug-info-slash.c"
+// LINUX: !DIFile(filename: "{{.*}}clang/test/CodeGen/debug-info-slash.c"
----------------
Does the test runner write the 'clang/test/CodeGen' path with forward slashes also on Windows?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:787
 
   StringRef PathRef(Asm->TM.Options.ObjectFilenameForDebug);
   llvm::SmallString<256> PathStore(PathRef);
----------------
This handles codeview. Does anything need to be done for dwarf on windows? mstorsjo might have input on that.


================
Comment at: llvm/test/DebugInfo/COFF/build-info.ll:13
 
+; Test path is canalized to windows backslash style when output object file name
+; is not starting with '/'.
----------------
s/canalized/canonicalized/


================
Comment at: llvm/test/DebugInfo/COFF/build-info.ll:18
+; RUN: cd %t-dir
+; RUN: llc -filetype=obj -mtriple i686-pc-windows-msvc %s -o ../build-info.o
+; RUN: llvm-readobj --codeview ../build-info.o | FileCheck %s --check-prefix=OBJ
----------------
Does this write the .o file into the test directory? I don't think that's allowed (it may be read-only). But the test could create another subdirectory under `%t-dir`.


================
Comment at: llvm/test/DebugInfo/COFF/build-info.ll:21
+
+; OBJ: ObjectName: ..\build-info.o
+
----------------
But in the `llc` invocation, the user wrote a relative path with a forward slash. What behavior do we want? What should happen if there are more then one slash - I think remove_dots just works on the first one?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147256/new/

https://reviews.llvm.org/D147256