ChuanqiXu marked an inline comment as done.
ChuanqiXu 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;
----------------
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. 


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

Reply via email to