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