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
