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

ChuanqiXu added a comment. In D139168#4119190 , @vvereschaka wrote: > @ChuanqiXu, > > would you provide a complete rollback for the changes and fix the > `C1689.cppm` test failures on the builders > > - > https://lab.llvm.org/buildbot/#/builders/119/bu

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

vvereschaka added a comment. @ChuanqiXu, would you provide a complete rollback for the changes and fix the `C1689.cppm` test failures on the builders - https://lab.llvm.org/buildbot/#/builders/119/builds/11935/steps/9/logs/FAIL__Clang__P1689_cppm - https://lab.llvm.org/buildbot/#/builders/60/

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

TWeaver added a comment. @ChuanqiXu Hi there, the revert of this patch has caused the following buildbot to start failing: https://lab.llvm.org/buildbot/#/builders/192/builds/267 It looks as though the removal of the // REQUIRES: !system-windows has caused the test to start running, and theref

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe1354763b6e6: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file… (authored by ChuanqiXu). Changed prior to commit: http

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

jansvoboda11 accepted this revision. jansvoboda11 added a comment. This revision is now accepted and ready to land. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139168/new/ https://reviews.llvm.org/D139168 ___ cfe-commits mailing list

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

ChuanqiXu added a comment. In D139168#4036850 , @tahonermann wrote: >> Header units have even more need to be involved in the build graph than >> named units. ODR violations and cache invalidation problems await anyone >> just winging it on header unit

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

jansvoboda11 added inline comments. Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:775 +static llvm::StringMap OSs; +std::unique_lock LockGuard(Lock); + How will this work when a different process tries to write the same file? C

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

tahonermann added a comment. > Header units have even more need to be involved in the build graph than named > units. ODR violations and cache invalidation problems await anyone just > winging it on header units (at least that's the understanding I've gotten > from SG15 meetings). I think that

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

Arthapz added a comment. > BTW, for header units, it is still under discussion that how should build > system and compiler interact about header units. It is still unclear whether > or not the header units should be transparent to build systems (and other > tools). On XMake we built our suppor

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

ChuanqiXu added a comment. > FWIW, the same with should happen with -MT. Yeah, it should be. Since the patch reuses the previous logics. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139168/new/ https://reviews.llvm.org/D139168 ___ cfe-commi

[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 discu

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

ChuanqiXu added a comment. > Hi, just wanted to say that i added support of these patch to XMake (i built > a LLVM with your 4 patches) and it work pretty well :) (except for header > units) @Arthapz Great to hear that! It is pretty important for us to know there are more build systems which a

[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 `clang-sca

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

Arthapz added a comment. Hi, just wanted to say that i added support of these patch to XMake and it work pretty well :) > xmake b -vD [ 0%]: generating.module.deps src

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

ChuanqiXu added a comment. >> In what mode will clang output #include information without erroring about >> missing .pcm files for import statements? -E -fdirectives-only perhaps? This >> further exacerbates the need for "fake" command lines and costs on platforms >> with expensive process exec

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

ChuanqiXu added a comment. > IMO, this is not suitable; there must be *one* depfile for Ninja to work > (otherwise build tools will need to manually collate all of this into one for > ninja. Yeah, but the producer of the compilation database is able to create a compilation db with the same `-M

[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 diffe

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

ChuanqiXu added a comment. @ben.boeckel I updated the test with compilation db. BTW, I am curious that according to https://reviews.llvm.org/D137534, cmake may not use clang-scan-deps with compilation db for P1689 . So this is a pure review comment and it is not

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

ChuanqiXu updated this revision to Diff 486472. ChuanqiXu added a comment. Add the test to generate make-style dependency file with compilation database. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139168/new/ https://reviews.llvm.org/D139168 Files: clang/include/clang/Tooling/Depe

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

ChuanqiXu added a comment. In D139168#4026799 , @ben.boeckel wrote: > In D139168#4025277 , @ChuanqiXu > wrote: > >> Currently we will detect `-MF` in the command line and we will write the >> make-format depend

[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` wit

[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)

ChuanqiXu added a comment. 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`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139168/new/ https://reviews.llvm.org/D139168 __

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

ChuanqiXu updated this revision to Diff 486196. ChuanqiXu added a comment. Update since https://reviews.llvm.org/D137534 changes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139168/new/ https://reviews.llvm.org/D139168 Files: clang/include/clang/Tooling/DependencyScanning/Dependenc

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

ChuanqiXu added a comment. @Bigcheese @jansvoboda11 gentle ping~ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139168/new/ https://reviews.llvm.org/D139168 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin

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

ChuanqiXu updated this revision to Diff 483083. ChuanqiXu added a comment. Rebase onto main. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139168/new/ https://reviews.llvm.org/D139168 Files: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h clang/lib/Tooling/D

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

ChuanqiXu added a comment. ping @Bigcheese @jansvoboda11 would you take a look at this if it will affect the existing users and the development? Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139168/new/ https://reviews.llvm.org/D139168 _

[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] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

ChuanqiXu 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: > 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)

ChuanqiXu updated this revision to Diff 480329. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139168/new/ https://reviews.llvm.org/D139168 Files: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h clang/lib/Tooling/D

[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 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)

ChuanqiXu 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: > > > For scanning, this cannot

[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. Th

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

ChuanqiXu 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: > For scanning, this cannot be the object file. The output needs to be th

[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 (or

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

ChuanqiXu created this revision. ChuanqiXu added reviewers: ben.boeckel, Bigcheese, jansvoboda11. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Required in https://reviews.llvm.org/D137534. The buil