ChuanqiXu added a comment. In D120540#3359454 <https://reviews.llvm.org/D120540#3359454>, @iains wrote:
> In D120540#3359446 <https://reviews.llvm.org/D120540#3359446>, @ChuanqiXu > wrote: > >> In D120540#3359394 <https://reviews.llvm.org/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` for C++20 coroutine before. >> >>> there is some handling of sources that should be changed accordingly. >> >> If I don't misunderstand, I guess we don't. Since all the code about C++20 >> modules are controlled by `CPlusPlusModules` variable. I think there >> wouldn't be codes controlled by `Cpp20` variables. > > Actually, the comment was intending to refer to the driver specifically - > since (for example) we have to disambiguate PCH jobs from C++20 Header Unit > jobs - in the Front End, as of now we have been using CPlusPlus modules as > the indicator for C++20 modules. There is still scope for confusion if that > is set at the same time as alternates... Oh, I didn't see Header Unit into details. I'm mainly focus on named module now. From my personal point, I feel header unit and named module are really not the same in many aspects so that we could handle them separately. >>> 2. I believe that it is a general objective of the tooling folks (roughly >>> SG15 participants) that C++20 modules should (eventually) be considered >>> automatic for C++20+ >> >> If I understand correctly, clang would enable C++20 modules by default if >> C++20 is enabled. > > Yes, exactly, as is the case right now. > >>> 3. There is at least one request from tooling folks that there should be an >>> option to disable modules (even when in C++20 mode); this is a practical >>> measure to avoid the case that it is impossible to build a TU because of >>> some potential modules-related bug ... >> >> I think `-fno-cxxmodules` could be the option now. > > so long as this is decoupled from any clang modules meanings? Yes, `fcxx-modules` only refers to C++20 modules now. >>> 4. IMO it becomes increasingly important to try and decouple the clang >>> modules from C++20 modules as much as possible. >> >> 100% agree. I think it should be beneficial to decouple them from command >> line, implementation and comments. (Now many comments are not precise. For >> example, when we talk about a `Module`. we are referring a module unit >> indeed. There are other examples.) > > It would be friendly to the user to reject command line options that are not > appropriate to the "current modules mode" - since there are ≈ 60+ > modules-related command line options it is already very easy to be confused. > To do this, the driver needs to establish the "current modules mode" early > (even before it builds jobs, since as noted above some jobs build differently > depending on the assumed mode). Totally agreed. It would be very hard process to disambuguate them since there are already users.. >>> So .. I was going to suggest that we might introduce -fmodules= {none, >>> clang, c++20, ...} >>> with defaults picked: >>> >>> fmodules => clang (i.e. the current meaning) >>> >>> !fmodules && C++20+ => c++20 (i.e. the objective of (2) above >>> Where there are other flags that imply C++20 that can switch c++20 mode >>> as well >>> >>> otherwise the default would be "none" >>> >>> .. this provides for (3) ... since -std=c++20 -fmodules=none could be used. >>> >>> I do not have a patch for this proposal as of this time (my current patches >>> assume (2) but do not meet the objective of (3)) >> >> The proposal is good to me though. I sent a patch >> (https://reviews.llvm.org/D113391) before to forbid the mixed use of clang >> modules and c++20 modules since it might be confusing if we use the >> combination `-fmodules -std=c++20`. But the comment shows that the current >> users of Clang Modules (mainly Google and Apple) wish a smooth switch from >> clang module to c++20 module. So I think the proposal which would break the >> current use cases would be not easy to land. >> (This patch wouldn't break any use case I think). > > Well, I think neither proposal breaks current use-cases - the selection of > defaults is designed to make current command lines do exactly the same as > they do now. > > My comments are not intended as a blocker for your patch - but simply to > offer a suggestion for a more generic handling of the same objective. Got it, thanks. My objective is relatively smaller. I just want to enable the use of C++20 modules for actual projects. (I heard that there are projects couldn't upgrade to C++20 due to a ABI break in std::string). Given the command line design of coroutines, I think this might be acceptable. 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.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
