Skip to content

C++20 modules available but __cpp_modules not defined #71364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
boris-kolpackov opened this issue Nov 6, 2023 · 8 comments
Open

C++20 modules available but __cpp_modules not defined #71364

boris-kolpackov opened this issue Nov 6, 2023 · 8 comments
Labels
c++20 clang:modules C++20 modules and Clang Header Modules

Comments

@boris-kolpackov
Copy link
Contributor

While Clang now enables C++20 modules by default with -std=c++20 and later, it does not appear to define the __cpp_modules feature test macros (which should have the value of 201907L; see https://en.cppreference.com/w/cpp/feature_test). I've tested this with up to Clang 18 pre-release (20231102103655+18839aec4ed1).

Is there a reason for omitting this macro for now or is it an oversight that could be fixed? In build2 we currently define this macro on the command line which adds quite a bit of noise.

@iains @ChuanqiXu9 @dwblaikie

@dtcxzyw dtcxzyw added c++20 clang:modules C++20 modules and Clang Header Modules and removed new issue labels Nov 6, 2023
@llvmbot
Copy link
Member

llvmbot commented Nov 6, 2023

@llvm/issue-subscribers-c-20

Author: Boris Kolpackov (boris-kolpackov)

While Clang now enables C++20 modules by default with `-std=c++20` and later, it does not appear to define the `__cpp_modules` feature test macros (which should have the value of `201907L`; see https://en.cppreference.com/w/cpp/feature_test). I've tested this with up to Clang 18 pre-release (`20231102103655+18839aec4ed1`).

Is there a reason for omitting this macro for now or is it an oversight that could be fixed? In build2 we currently define this macro on the command line which adds quite a bit of noise.

@iains @ChuanqiXu9 @dwblaikie

@llvmbot
Copy link
Member

llvmbot commented Nov 6, 2023

@llvm/issue-subscribers-clang-modules

Author: Boris Kolpackov (boris-kolpackov)

While Clang now enables C++20 modules by default with `-std=c++20` and later, it does not appear to define the `__cpp_modules` feature test macros (which should have the value of `201907L`; see https://en.cppreference.com/w/cpp/feature_test). I've tested this with up to Clang 18 pre-release (`20231102103655+18839aec4ed1`).

Is there a reason for omitting this macro for now or is it an oversight that could be fixed? In build2 we currently define this macro on the command line which adds quite a bit of noise.

@iains @ChuanqiXu9 @dwblaikie

@ChuanqiXu9
Copy link
Member

The major reason is that we haven't marked the status of Modules as complete now. See https://clang.llvm.org/cxx_status.html

The reason behind this may be a different strategy to claim a feature as stable. For example, the support of coroutines in clang is marked as partial before Mar this year: https://discourse.llvm.org/t/rfc-could-we-mark-coroutines-as-unreleased-now/69220, while it is already used in production for years.

@boris-kolpackov
Copy link
Contributor Author

The major reason is that we haven't marked the status of Modules as complete now.

This feels inconsistent to me: you enable modules by default in -std=c++20 (and have dropped -fmodules-ts) but you don't signal in any way that they are available.

Perhaps a compromise would be to define this macro but with a value that is an earlier date. I believe GCC does it this way.

@ChuanqiXu9
Copy link
Member

The major reason is that we haven't marked the status of Modules as complete now.

This feels inconsistent to me: you enable modules by default in -std=c++20 (and have dropped -fmodules-ts) but you don't signal in any way that they are available.

Sounds good. I'd like to define the macro later if no objection comes in.

@AaronBallman
Copy link
Collaborator

I think there's evidence that defining the macro at all when modules is not complete will cause problems in practice: https://sourcegraph.com/search?q=context:global+%23if+defined%28__cpp_modules%29+-file:.*test.*+-file:.*clang.*+-file:.*gcc.*&patternType=standard&case=yes&sm=1&groupBy=repo

In general, I think we don't want to define feature test macros until we're comfortable claiming we fully support the functionality. The one big exception to this is when system headers (mis)use the feature testing macros in a way that's critical for us to support.

@ChuanqiXu9
Copy link
Member

ChuanqiXu9 commented Dec 20, 2024

BTW, the list of missing features in my mind is collected in #112295

And I am open for whether to not to define the macro now. Since although it is not complete in fact, modules are already being used. For example, this is an incomplete list I searched few days ago for the use of boost in modules:

https://github.com/infiniflow/infinity/blob/fa922afd58c973a9a317d2c09ad8cea8984af91e/src/common/boost.cppm#L17
https://github.com/ossia/score/blob/0ce6f62a97651a1248a92b3e13ce6ede10341d1f/src/plugins/score-plugin-avnd/halp.cppm#L74
https://github.com/RichardLuo0/make-dot-cpp/blob/e2ef10fd43060a2b56fc8f87f1407e4e3b1c2b8e/src/utils/module.cppm#L4
https://github.com/slaakko/cmajor-mod/blob/32e6eb8dc099b395fa751340de96f47817f92977/cmajor/system-x/machine/clock.cppm#L7
https://github.com/df-com/dragonfly/blob/9dfcbc1153f292eb8581f1df7459600cc331c690/dragonfly/Chart.cppm#L9
https://github.com/nofe1248/Helium/blob/92ec4452b8c05e6d86c972f843196a4392a7a6da/Events/EventBus/Helium.Events.EventBus.cppm#L20
https://github.com/LagrangeDev/liblagrange/blob/10e3b48f8d47fd65de00b14cfed64c0be18c083d/src/base/transport/service.cppm#L7
https://github.com/yudaichen/feature-db/blob/e94ade969af682ed5dd25d70bf282ba39253cbf0/src/common/small_utils.cppm#L7

So my logic is, if it is already used, it looks fine to me to define the feature macro. And similarly, if we don't want to define the macro now, I don't feel bad too. Since it looks it doesn't prevent people using it : )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++20 clang:modules C++20 modules and Clang Header Modules
Projects
None yet
Development

No branches or pull requests

6 participants