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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
