[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ben.boeckel added a comment. In D134267#3876071 , @dblaikie wrote: > I'm getting a bit exhausted with all the words involved here & not sure how > to simplify/clarify this. > > If @ben.boeckel has particular use cases, it might be easier for him to be

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

ben.boeckel added inline comments. Comment at: clang/test/ClangScanDeps/P1689.cppm:9 +// +// Check the seperated dependency format. +// RUN: clang-scan-deps -format=p1689 --p1689-targeted-file-name=%t/M.cppm --p1689-targeted-output=%t/M.o \ jansvoboda11 wrote:

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

ben.boeckel added a comment. In D137534#4014571 , @ChuanqiXu wrote: >> Why is it necessary to add new command-line flags for this? Can't the input >> and output be inherited from the specified Clang command line? Would such >> command line make sense?

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

ben.boeckel added a comment. In D139168#4030425 , @ChuanqiXu wrote: > BTW, if you are interested, it should be possible to use clang-scan-deps to > get the make-style format information. I want a *single* depfile for *anything* scanned by the

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

ben.boeckel added a comment. In D139168#4025277 , @ChuanqiXu wrote: > Currently we will detect `-MF` in the command line and we will write the > make-format dependency output to the corresponding file once we find `-MF`. Which is fine, but docs need to

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

ben.boeckel added a comment. In D139168#4025277 , @ChuanqiXu wrote: > Currently we will detect `-MF` in the command line and we will write the > make-format dependency output to the corresponding file once we find `-MF`. When using `clang-scan-deps`

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

ben.boeckel added a comment. In D137534#4024022 , @jansvoboda11 wrote: > While I was not suggesting using compilation database instead of the approach > taken by this patch, I appreciate your explanation. But it still leaves me > wondering whether

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

ben.boeckel added a comment. In D137534#4024249 , @jansvoboda11 wrote: > I'm getting confused by the semantics of `--p1689-targeted-output=`. In > D137527 , it's used to populate > `CompilerCommand::Output` which is

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

ben.boeckel added a comment. In D137534#4037064 , @jansvoboda11 wrote: > Another thing to be aware of is that the scanner is tuned for scanning > multiple TUs. Single `clang-scan-deps` invocation maintains a shared > in-memory cache of the filesystem

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

ben.boeckel added inline comments. Comment at: clang/test/ClangScanDeps/P1689.cppm:155 +// CHECK-MAKE: [[PREFIX]]/impl_part.o: +// CHECK-MAKE: [[PREFIX]]/impl_part.cppm ChuanqiXu wrote: > ben.boeckel wrote: > > For scanning, this cannot be the object file.

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

ben.boeckel added inline comments. Comment at: clang/test/ClangScanDeps/P1689.cppm:155 +// CHECK-MAKE: [[PREFIX]]/impl_part.o: +// CHECK-MAKE: [[PREFIX]]/impl_part.cppm ChuanqiXu wrote: > ben.boeckel wrote: > > ChuanqiXu wrote: > > > ben.boeckel wrote: > > >

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

ben.boeckel added inline comments. Comment at: clang/test/ClangScanDeps/P1689.cppm:155 +// CHECK-MAKE: [[PREFIX]]/impl_part.o: +// CHECK-MAKE: [[PREFIX]]/impl_part.cppm ben.boeckel wrote: > ChuanqiXu wrote: > > ben.boeckel wrote: > > > ChuanqiXu wrote: > > >

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

ben.boeckel added a comment. This worked for CMake in a previous version, so is good enough on the functionality front. I'll retry with the current state to verify. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139168/new/ https://reviews.llvm.org/D139168

[PATCH] D137059: [Driver] [Modules] Introduce -fsave-std-c++-module-file= to specify the path of the module file (2/2)

ben.boeckel added a comment. In D137059#3934448 , @dblaikie wrote: > I'm still curious what about the details of other compilers - I think from > the sounds of it, @iains suggested GCC doesn't support this yet so we'll need > to pick/name the flag

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/3)

ben.boeckel added inline comments. Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:192 +"p1689-targeted-directory", llvm::cl::Optional, +llvm::cl::desc("Only supported for P1689, the targeted directory of which " + "the dependencies are to be

[PATCH] D137058: [Driver] [Modules] Support -fsave-std-c++-module-file (1/2)

ben.boeckel added a comment. I'm not too concerned with the spelling of the flag myself. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137058/new/ https://reviews.llvm.org/D137058 ___ cfe-commits

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/3)

ben.boeckel added inline comments. Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:197 +llvm::cl::opt P1689TargetedCommand( +"p1689-targeted-command", llvm::cl::Optional, +llvm::cl::desc("Only supported for P1689, the targeted command of which "

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/3)

ben.boeckel added inline comments. Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:197 +llvm::cl::opt P1689TargetedCommand( +"p1689-targeted-command", llvm::cl::Optional, +llvm::cl::desc("Only supported for P1689, the targeted command of which "

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/3)

ben.boeckel added inline comments. Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:197 +llvm::cl::opt P1689TargetedCommand( +"p1689-targeted-command", llvm::cl::Optional, +llvm::cl::desc("Only supported for P1689, the targeted command of which "

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/3)

ben.boeckel added a comment. So a few things I see when trying to put this into CMake: - P1689 output is only to `stdout`? - Is there a way to get `-MF` output for the files read during scanning? This is useful to know that "header X changed, scanning may have

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

ben.boeckel added inline comments. Comment at: clang/test/ClangScanDeps/P1689.cppm:155 +// CHECK-MAKE: [[PREFIX]]/impl_part.o: +// CHECK-MAKE: [[PREFIX]]/impl_part.cppm For scanning, this cannot be the object file. The output needs to be the P1689 output

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

ben.boeckel added a comment. In D139168#4034885 , @ChuanqiXu wrote: > @Arthapz Great to hear that! It is pretty important for us to know there are > more build systems which are using these functionality. BTW, for header > units, it is still under

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

ben.boeckel added a comment. In D139168#4027637 , @ChuanqiXu wrote: > Currently, clang-scan-deps won't check for this. If we have multiple command > lines with different `-MF` value, the make-style dependency information will > be written to these

[PATCH] D137059: [Driver] [Modules] Introduce -fsave-std-c++-module-file= to specify the path of the module file (2/2)

ben.boeckel added a comment. In D137059#3904256 , @ChuanqiXu wrote: > In my mind, it is OK for CMake to support one-phase compilation model in the > short term. And the fact that clang also supports the 2-phase compilation > wouldn't affect CMake. Do

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ben.boeckel added a comment. In D134267#3904257 , @ChuanqiXu wrote: > BTW, this patch should be discarded. And now we're pursuing D137059 > and D137058 > , where there is no modules

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ben.boeckel added a comment. FWIW, I was able to make CMake's module test suite pass with this patch on top of you MyP1689 branch on your Github fork. I also added some diffs to help clean up output files. I'll try and get it to work with the replacement patches as well, but I just want this

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

ben.boeckel added a comment. I tried applying this patch to your MyP1689 branch (fixing conflicts with attempts there), but it seems that something isn't being plumbed properly: clang-15: error: unknown argument:

[PATCH] D137058: [Driver] [Modules] Support -fsave-std-c++-module-file (1/2)

ben.boeckel added a comment. In D137058#3896999 , @dblaikie wrote: > Could you expound a bit on why "write the .pcm to the same path as the .o" > isn't sufficient? (like Split DWARF does this, for instance - not sure we > have lots of other things to

[PATCH] D137059: [Driver] [Modules] Introduce -fsave-std-c++-module-file= to specify the path of the module file (2/2)

ben.boeckel added a comment. There is another motivating factor for 1-phase: the build graph is far simpler. With 2-phase, CMake will have to write out rules to perform: - source -> .bmi - .bmi -> .withbmi.o - source -> .o because we do not know if a BMI is needed or not. If it isn't we use