[PATCH] D118893: [C++20][Modules][1/8] Track valid import state.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu accepted this revision.
ChuanqiXu added a comment.

LGTM.



================
Comment at: clang/lib/Sema/SemaModule.cpp:144
     GlobalModuleFragment = ModuleScopes.back().Module;
 
   // In C++20, the module-declaration must be the first declaration if there
----------------
iains wrote:
> ChuanqiXu wrote:
> > I feel better to add an assertion here:
> > ```
> > assert(SeenGMF == GlobalModuleFragment && "msg);
> > ```
> we also have to check that we are in C++20 modules mode, since implicit global module fragments are allowed elsewhere - the test below already guards on C++20 modules.
> 
> I've made this change, although the assert seems quite heavyweight.
nit: Is the 
```
(SeenGMF == (GlobalModuleFragment != nullptr))
```
not equal to:
```
SeenGMF == GlobalModuleFragment
``` 
? Or we could add a cast here:
```
SeenGMF == (bool) GlobalModuleFragment
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118893