rnk added inline comments.
================ Comment at: clang/include/clang/Basic/CodeGenOptions.h:212 + /// Output filename used in the COFF debug information. + std::string COFFOutputFilename; + ---------------- aganea wrote: > rnk wrote: > > Let's bikeshed the name a bit. This thing is the `-o`/`/Fo` value, plus > > some processing. It can be symbolic, as in `-`. It could be the name of a > > bitcode file with `-flto`. It could be the name of an assembly file if you > > do `clang -S --target=x86_64-windows-msvc -g t.cpp -o t.s`. It could be the > > name of an ELF file if you try hard. I think in any of these cases where > > the user directly emits something that isn't a COFF object, it's OK to use > > that name for the S_OBJNAME record. > > > > What it is, is the name of the compiler's output file, as we would like it > > to appear in debug info. With that in mind, here are some ideas: > > - CodeViewObjectName: very directly referring to S_OBJNAME > > - ObjectFilename: very general > > - ObjectFilenameForDebug: generalizing over cv/dwarf > > - OutputFilenameForDebug: generalizing over assembly, bc, and obj > > > > I think I like ObjectFilenameForDebug best. DebugObjectFilename seems like > > a no-go, since that sounds like the dwo file name. > > > > --- > > > > This brings me to the case of `-save-temps` / `/Fa`. In these cases, the > > compile action is broken into pieces, and the assembler is invoked in a > > follow-up process. Does that mean the driver needs to pass an extra flag > > along to the -cc1 action? That would be a bummer. > > > I think we should fix `-save-temps` and `/Fa`, thanks for raising that! Would > you mind if I added a new cc1 flag for that purpose? Something like > `-target-file-name`. > When using `-save-temps`, each cc1 command doesn't know about the final > filename, except for the last cc1 command. We would need to provide that name > somehow when passing `-S`. Yes, I think we'll need a new cc1 flag. I'd avoid "target" in the flag name, since it makes me thing of target as in platform, OS, ISA, etc. Maybe just -object-file-name? It's very boring, and reasonably specific. ================ Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:752 +static void unescapeSlashes(SmallVectorImpl<char> &Str) { + auto Read = Str.begin(); ---------------- aganea wrote: > rnk wrote: > > This isn't unescaping them, it's just canonicalizing double slashes to one > > slash, right? Would `llvm::sys::native` suffice? > `llvm::sys::path::native()` doesn't remove consecutive (back)slashes. I think > @zturner's main intent was when debugging in Visual Studio with arguments > from LIT tests, they sometimes contain double backslashes. I see. I think "unescape" shouldn't be in the name, this isn't about escape characters, it's about cleaning up or canonicalizing the path. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43002/new/ https://reviews.llvm.org/D43002 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits