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

Chuanqi Xu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 8 19:21:16 PST 2021


ChuanqiXu added a comment.

In D113391#3115410 <https://reviews.llvm.org/D113391#3115410>, @tschuett wrote:

> It would enforce that there is exactly one module mode live.

Yeah. But my consideration is that it would enforce another variable and it couldn't eliminate the two existing variables. So it looks redundant to me. We have made this check in driver and frontend part. So it looks sufficient to me.
I admit that the current method has limitations. For example, it is possible that both `ClangModules` and `CPlusPlusModules` could be wrote as true  in Lexer part and Sema part (Maybe we could add an assertion in `hasModules()`). And your suggestion could solve this. But I have another question about the implementation, where should we put this variable and when should we fill the value for it? I failed to use a new variable in LangOpts and initialize it in constructor of LangOpts since both `ClangModules` and `CPlusPlusModules` is zero.
And even in this suggestion, user could still access `ClangModules` and `CPlusPlusModules`. And they could rewrite them too. So I am wondering if it would be more confusing.


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