[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

ChuanqiXu added a comment. @aprantl Thanks for the reverting. I never image `-fcxx-modules` would refer to Clang C++ modules. So the problem becomes more complicated. In D120540#3551169 , @iains wrote: > I guess Chuanqi's TZ is in sleep mode at the mome

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

iains added a comment. I guess Chuanqi's TZ is in sleep mode at the moment, so the revert makes sense. Please let's not use -fmodules-ts to mean anything - we want to phase it out and make it a NOP (we are implementing the standardised modules; I do not see anyone putting effort into finishing

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

aprantl added a comment. I'm going to revert the patch now. This is not just breaking LLDb, but also clang itself on Darwin platforms. I think we need to be more careful to separate out the enabling of Clang C++ modules and C++20 modules. Either by having `-fmodules-ts` control the `HaveModules

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

iains added a comment. as noted before, IMO this is all a bit tangled at the moment. Probably, a good start would be to make the driver and the FE behave the same way for the flags -- at the moment C++20 jams on fmodules and fcxx-modules (meaning that there's no way to decouple them in the FE)

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

aprantl added a comment. In any case, I would appreciate it if we could revert this patch until we found a solution! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120540/new/ https://reviews.llvm.org/D120540 __

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

aprantl added a comment. Should we introduce a `-fno-modules-ts` option on top? Any better suggestions? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120540/new/ https://reviews.llvm.org/D120540 ___ cfe-

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

aprantl added a comment. I think the problem might be that previously on Darwin `-fcxx-modules` was used to turn on C++ Clang modules (which was not implied by `-fmodules`) without turning of C++20 (ts) modules, and after this patch `-fcxx-modules` implies `-fmodules-ts`. Does that sound plausi

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

aprantl added a comment. https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/44167/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120540/new/ https://reviews.llvm.org/D120540 ___ cfe-commits mailin

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

aprantl added a comment. This patch broke a whole bunch tests in the LLDB testsuite. I'm trying to figure out what exactly the semantics of the change are, particularly on Darwin, where `-fmodules` doesn't imply `-fcxx-modules`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

DanielMcIntosh-IBM added inline comments. Comment at: clang/test/Driver/modules.cpp:79-86 +// RUN: %clang++ -fcxx-modules -std=c++17 -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-CXX17-MODULES +// CHECK-CXX17-MODULES: "-fcxx-modules" +// RUN: %clang++ -fcxx-modules -std=c++14

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

DanielMcIntosh-IBM added inline comments. Comment at: clang/test/Driver/modules.cpp:79-86 +// RUN: %clang++ -fcxx-modules -std=c++17 -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-CXX17-MODULES +// CHECK-CXX17-MODULES: "-fcxx-modules" +// RUN: %clang++ -fcxx-modules -std=c++14

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG99eca8353808: [Driver] Enable to use C++20 standalne by -fcxx-modules (authored by ChuanqiXu). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

ChuanqiXu added a comment. @iains Do you agree to submit this one first? Since all the discussions/questions I see in this page now it not about the patch itself. The patch itself should be good personally I thought. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120540/new/ https://re

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

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

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

ChuanqiXu updated this revision to Diff 419649. ChuanqiXu added a reviewer: MaskRay. ChuanqiXu added a comment. Herald added a subscriber: StephenFan. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120540/new/ https://reviews.llvm.org/D120540 Files: clang/include/cla

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

MaskRay added inline comments. Comment at: clang/test/Driver/modules.cpp:79 +// +// RUN: %clang -fcxx-modules -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-CXX-MODULES +// CHECK-CXX-MODULES: "-fcxx-modules" You need to specify a pre-20 -stdc to make the test

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

ChuanqiXu added a comment. Herald added a subscriber: MaskRay. @rsmith @Bigcheese gentle ping~ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120540/new/ https://reviews.llvm.org/D120540 ___ cfe-commits m

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

iains added a comment. In D120540#3384644 , @ChuanqiXu wrote: > @rsmith told me that the ideal situation would combine C++20 modules and > clang modules together in D113391 Maybe I understand something slightly differe

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

ChuanqiXu added a subscriber: rsmith. ChuanqiXu added a comment. @rsmith told me that the ideal situation would combine C++20 modules and clang modules together in D113391 I think the most important thing here is to get in consensus for the module status. Here

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

iains added a comment. I'm not sure it is exactly chaos, but it is certainly fragile and somewhat hard(er than necessary) to maintain. We (@ChuanqiXu and I at least) agree that there should be some way to make "which modules mode" unambiguous in both the driver and the compiler (I think we're

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

ChuanqiXu added a comment. In D120540#3381905 , @jansvoboda11 wrote: > I agree. My understanding is that `-fcxx-modules` enables Clang modules that > don't interact with C++20 modules. @Bigcheese, any thoughts? Oh, if it is true, then it is in a chaos

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

jansvoboda11 added a comment. I agree. My understanding is that `-fcxx-modules` enables Clang modules that don't interact with C++20 modules. @Bigcheese, any thoughts? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120540/new/ https://reviews.llvm.

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

iains added a comment. I am still concerned that there is an expectation that. the fcxx-modules option is connected with clang modules. .. see, for example: https://github.com/llvm/llvm-project/blob/d90d45fc9029cc7dbb6d44798f51131df6b2eef1/clang/lib/Driver/ToolChains/Clang.cpp#L3579 and https:

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

ChuanqiXu added a comment. @iains ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120540/new/ https://reviews.llvm.org/D120540 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

ChuanqiXu added a comment. In D120540#3359454 , @iains wrote: > In D120540#3359446 , @ChuanqiXu > wrote: > >> In D120540#3359394 , @iains wrote: >> >>> 1. I agree 100% th

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

iains added a comment. In D120540#3359446 , @ChuanqiXu wrote: > In D120540#3359394 , @iains wrote: > >> 1. I agree 100% that the driver needs to be able to process in "c++20 >> modules mode"; > > Yeah, not all th

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

ChuanqiXu added a comment. In D120540#3359394 , @iains wrote: > 1. I agree 100% that the driver needs to be able to process in "c++20 modules > mode"; Yeah, not all the projects could be able to run in c++20 mode. We use `-std=c++17` + `-fcoroutines-ts

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

iains added a comment. of course, this meets your objective too - since you could put -fmodules=cxx20 (even without -std=c++20) .. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120540/new/ https://reviews.llvm.org/D120540

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

iains added a comment. 1. I agree 100% that the driver needs to be able to process in "c++20 modules mode"; there is some handling of sources that should be changed accordingly. 2. I believe that it is a general objective of the tooling folks (roughly SG15 participants) that C++20 modules shoul

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

ChuanqiXu created this revision. ChuanqiXu added reviewers: jansvoboda11, sammccall, urnathan, Quuxplusone. ChuanqiXu added a project: clang. Herald added a subscriber: dang. ChuanqiXu requested review of this revision. Herald added a subscriber: cfe-commits. This patch allows user to use C++20 mo