[PATCH] D113391: [Modules] Don't support clang module and c++20 module at the same time

Richard Smith - zygoloid via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 9 11:30:48 PST 2021


rsmith requested changes to this revision.
rsmith added a comment.
This revision now requires changes to proceed.

The ideal situation here would be to have a single semantic model that is a superset of the various combinations of behaviors we want. Further entrenching Clang modules and C++20 modules as being different things would be a backwards step, and would be harmful for users of Clang modules who want to incrementally adopt C++20 modules. We shouldn't have a separate language option flag for "clang modules" -- that should be a different kind of module, not a different compilation mode; if a clang header module needs different treatment from a C++20 header unit, we should base that on the kind of the current `clang::Module`, not on a language option.

In terms of what the various `LangOptions` flags mean:

`LangOptions::Modules` corresponds to the expectation that modular headers will be imported rather than entered textually.
`LangOptions::ModulesTS` enables C++ Modules TS language syntax. This corresponds to `-fmodules-ts`. We should probably deprecate it.
`LangOptions::CPlusPlusModules` enables C++20 modules syntax. This is enabled by `-std=c++20`.
`LangOptions::ModulesLocalVisibility` indicates that in this compilation, visibility is not monotonically increasing. This corresponds to `-fmodules-local-submodule-visibility`. This is the case when we might compile multiple source files or header units into a single AST, and we want to reset the visibility state at the end of each  such unit. For example, this happens when building a Clang module containing multiple headers. This really should be enabled automatically when appropriate, but for now it's enabled by default with `CPlusPlusModules` and `ModulesTS`.

The only combination that's incompatible here is `ModulesTS` plus `CPlusPlusModules`, because they have overlapping but different syntax.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113391/new/

https://reviews.llvm.org/D113391



More information about the cfe-commits mailing list