ChuanqiXu added inline comments.
================ Comment at: clang/lib/Driver/Driver.cpp:5541-5542 + // + // FIXME: Do we need to handle the case that the calculated output is + // conflicting with the specified output file or the input file? + if (!AtTopLevel && isa<PrecompileJobAction>(JA) && ---------------- tahonermann wrote: > tahonermann wrote: > > ChuanqiXu wrote: > > > dblaikie wrote: > > > > tahonermann wrote: > > > > > ChuanqiXu wrote: > > > > > > dblaikie wrote: > > > > > > > Do we do that for `-o` today? (eg: if you try to `-o` and specify > > > > > > > the input file name, such that the output would overwrite the > > > > > > > input, what happens? I'm not sure - but I guess it's OK to do > > > > > > > whatever that is for this case too) > > > > > > > Do we do that for -o today? (eg: if you try to -o and specify the > > > > > > > input file name, such that the output would overwrite the input, > > > > > > > what happens? I'm not sure - but I guess it's OK to do whatever > > > > > > > that is for this case too) > > > > > > > > > > > > In this case, the input file will be overwrite and no warning > > > > > > shows. So it looks like we didn't take special treatment here. So I > > > > > > remove the FIXME. > > > > > Basing the location of the module output on the presence of `-o` > > > > > sounds confusing to me. Likewise, defaulting to writing next to the > > > > > input file as opposed to the current directory (where a `.o` file is > > > > > written by default) sounds wrong. I think this option should be > > > > > handled similarly to `-o`; it should accept a path and: > > > > > - If an absolute path is provided, write to that location. > > > > > - If a relative path is provided, write to that location relative > > > > > to the current working directory. > > > > > Leave it up to the user or build system to ensure that the `.o` and > > > > > `.pcm` file locations coincide if that is what they want. In general, > > > > > I don't expect colocation of `.o` and `.pcm` files to be what is > > > > > desired. > > > > > > > > > > > > > > @tahonermann there's precedent for this with Split DWARF (.dwo files go > > > > next to the .o file) & I'd argued for possibly only providing this > > > > behavior, letting consumers move files elsewhere if they needed to, but > > > > got voted down there. > > > > > > > > I do think this is a reasonable default, though. Users and build > > > > systems have the option to pass a path to place the .pcm somewhere else > > > > (in the follow-up patch to this one). > > > @tahonermann here is another patch which implements the behavior you > > > described: https://reviews.llvm.org/D137059 > > I'm ok with a default that consistently writes the PCM relative to the > > location of the `.o` file (if not overridden with an absolute path). What > > I'm not ok with is having the default be next to the `.o` file if `-o` is > > specified and next to the input file if `-o` is not specified. I don't > > think writing the PCM relative to the input file is a good default in any > > case. If `-o` is not specified, then I think it should be written relative > > to the current working directory; which matches where the `.o` file will be > > written. > Summary of the consensus of the Clang Modules WG: > # If `-fmodule-output=` is provided, the PCM is written to the indicated > file (relative to the current working directory for a relative path). > # If `-fmodule-output` is provided, the PCM is written relative to the > location of the `.o` file (the current working directory by default and the > location indicated by `-o` otherwise). Done. 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
