ChuanqiXu added a comment.
I don't remember clearly if we have a test for the initializer in the
implementation unit. Ignore this if we already have one.
================
Comment at: clang/include/clang/Basic/Module.h:137
ImplicitGlobalModuleFragment,
};
----------------
We may be able to save 1 bit for ModuleKind by adding two field bits to Module:
`IsImplementation` and `IsPartition`. Then we can remove
`ModulePartitionInterface` and `ModulePartitionImplementation`. Then let's
rename `ModuleInterfaceUnit` to `NamedModuleUnit`.
So we can judge module interface unit, implementation unit, module partition
interface and module partition implementation by `NamedModuleUnit ` and the two
bits.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:1672-1677
+ // A module implementation unit has visibility of the decls in its implicitly
+ // imported interface.
+ if (getLangOpts().CPlusPlusModules && NewM && OldM &&
+ NewM->Kind == Module::ModuleImplementationUnit &&
+ OldM->Kind == Module::ModuleInterfaceUnit && NewM->Name == OldM->Name)
+ return false;
----------------
I feel slightly better to add a field in Sema:
```
Module *PrimaryModuleInterface = nullptr; // Only valid if we're parsing a
module unit.
```
Then we can avoid the string compare here. Also we can add it to
`Sema::isUsableModule`. Now `Sema::isUsableModule` works because it falls to
the string comparison.
================
Comment at: clang/lib/Sema/SemaModule.cpp:302
+ Module *Mod; // The module we are creating.
+ Module *Interface = nullptr; // The interface for an implementation.
switch (MDK) {
----------------
Then we can avoid to declare this if we have `Sema::PrimaryModuleInterface `
================
Comment at: clang/lib/Sema/SemaModule.cpp:408-409
+ // module, if any.
+ if (!ModuleScopes.empty())
+ Context.addModuleInitializer(ModuleScopes.back().Module, Import);
+
----------------
ModuleScopes can't be empty here.
================
Comment at: clang/test/CXX/module/basic/basic.def.odr/p4.cppm:155-156
+
+ // FIXME: Issue #61427 Internal-linkage declarations in the interface TU
+ // should not be not visible here.
(void)&static_var_module_linkage; // FIXME: Should not be visible here.
----------------
nit: I am a little bit tired to add FIXME in the tests... let's not try to add
more...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126959/new/
https://reviews.llvm.org/D126959
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
