dblaikie added inline comments.
================ Comment at: clang/lib/Driver/Driver.cpp:5736 + C.getArgs().hasArg(options::OPT_fmodule_output) && + C.getArgs().hasArg(options::OPT_o)) { + SmallString<128> OutputPath; ---------------- ChuanqiXu wrote: > dblaikie wrote: > > tahonermann wrote: > > > dblaikie wrote: > > > > ChuanqiXu wrote: > > > > > dblaikie wrote: > > > > > > Is there some way we can avoid this (`-fmodule-output -o ...`) > > > > > > being a special case? It'd be good if we could use a single common > > > > > > implementation regardless of whether `-o` is used (ie: Figure out > > > > > > the output file name (whether it's implicitly determined by clang, > > > > > > in the absence of `-o`, or explicitly provided by the user through > > > > > > `-o`) and then replace the suffix with `pcm`)? > > > > > I guess we can't do it or we can't do it easily. Otherwise the .pcm > > > > > file will always lives in the same directory with the input file. > > > > > I guess we can't do it or we can't do it easily. Otherwise the .pcm > > > > > file will always lives in the same directory with the input file. > > > > > > > > I don't follow/understand. What I mean is, I'd like it, if possible, > > > > this was implemented by something like "find the path for the .o file > > > > output, then replace the extension with .pcm". > > > > > > > > I worry atht code like this that recomputes something similar to the > > > > object output path risks getting out of sync with the actual object > > > > path. > > > That is certainly a valid concern and I agree it would be better if the > > > shared output path is computed exactly once. If that would require > > > significant change, then tests to ensure consistent behavior would be the > > > next best thing. I'm not sure what infrastructure is in place for > > > validation of output file locations though. > > Well, I guess the Split DWARF file naming isn't fundamentally better - it > > looks at the OPT_O argument directly too: > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/CommonArgs.cpp#L1250 > > > > Perhaps we could at least reuse this same logic - make it a reusable > > function in some way? (for instance, it checks `-c` too, which seems > > relevant - we wouldn't want to name the .pcm after the linked executable > > name, right? > > > > Perhaps we could at least reuse this same logic - make it a reusable > > function in some way? > > Done. It looks better now. > > > (for instance, it checks -c too, which seems relevant - we wouldn't want to > > name the .pcm after the linked executable name, right? > > Oh, right. Although the previous conclusion is that if `-o` is specified, the > .pcm file should be in the same dir with the output. But it is indeed weird > that the .pcm file are related to the linked executable if we thought it > deeper. Ah, I was hoping it could reuse the same code, rather than duplicate it - any chance it could be refactored into a common implementation between Split DWARF and modules? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137058/new/ https://reviews.llvm.org/D137058 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits