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

[Iain Sandoe via Phabricator via cfe-commits]

iains marked 4 inline comments as done.
iains added inline comments.


================
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
----------------
ChuanqiXu wrote:
> 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
> ```
> nit: Is the 

> ```
> SeenGMF == GlobalModuleFragment
> ``` 

clang complains of pointer / integer comparison.
I guess we could use the cast.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118893