aaron.ballman added inline comments.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16155
+ if (getLangOpts().CPlusPlusModules) {
+ auto *GlobalModule =
+ PushGlobalModuleFragment(ExternLoc, /*IsImplicit=*/true);
----------------
`Module *` instead of `auto *` (the type is not spelled out in the
initialization).
================
Comment at: clang/lib/Sema/SemaModule.cpp:71
// module-private (though they do not have module linkage).
- auto &Map = PP.getHeaderSearchInfo().getModuleMap();
- auto *GlobalModule = Map.createGlobalModuleFragmentForModuleUnit(ModuleLoc);
- assert(GlobalModule && "module creation should not fail");
-
- // Enter the scope of the global module.
- ModuleScopes.push_back({});
- ModuleScopes.back().BeginLoc = ModuleLoc;
- ModuleScopes.back().Module = GlobalModule;
- VisibleModules.setVisible(GlobalModule, ModuleLoc);
+ auto *GlobalModule =
+ PushGlobalModuleFragment(ModuleLoc, /*IsImplicit=*/false);
----------------
`Module *`
================
Comment at: clang/lib/Sema/SemaModule.cpp:707-708
+ bool IsImplicit) {
+ auto &Map = PP.getHeaderSearchInfo().getModuleMap();
+ auto *GlobalModule = Map.createGlobalModuleFragmentForModuleUnit(BeginLoc);
+ assert(GlobalModule && "module creation should not fail");
----------------
Please spell these out as well.
================
Comment at: clang/lib/Sema/SemaModule.cpp:712-715
+ ModuleScopes.push_back({});
+ ModuleScopes.back().BeginLoc = BeginLoc;
+ ModuleScopes.back().Module = GlobalModule;
+ ModuleScopes.back().ImplicitGlobalModuleFragment = IsImplicit;
----------------
Can we use `emplace_back()` and construct in place rather than constructing
piecemeal?
================
Comment at: clang/lib/Sema/SemaModule.cpp:722-723
+void Sema::PopGlobalModuleFragment() {
+ assert(!ModuleScopes.empty() && getCurrentModule().isGlobalModule() &&
+ "left the wrong module scope, which is not global module fragment");
+ ModuleScopes.pop_back();
----------------
FWIW, this seems to be failing to compile according to the precommit CI.
```
FAILED: tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaModule.cpp.o
CCACHE_CPP2=yes CCACHE_HASHDIR=yes /usr/bin/ccache /usr/bin/clang++
-DBUILD_EXAMPLES -DCLANG_ROUND_TRIP_CC1_ARGS=ON -DGTEST_HAS_RTTI=0 -D_DEBUG
-D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS
-D__STDC_LIMIT_MACROS -Itools/clang/lib/Sema
-I/var/lib/buildkite-agent/builds/llvm-project/clang/lib/Sema
-I/var/lib/buildkite-agent/builds/llvm-project/clang/include
-Itools/clang/include -Iinclude
-I/var/lib/buildkite-agent/builds/llvm-project/llvm/include -gmlt -fPIC
-fvisibility-inlines-hidden -Werror=date-time
-Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter
-Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic
-Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough
-Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor
-Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion
-Wmisleading-indentation -fdiagnostics-color -ffunction-sections
-fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3
-DNDEBUG -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT
tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaModule.cpp.o -MF
tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaModule.cpp.o.d -o
tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaModule.cpp.o -c
/var/lib/buildkite-agent/builds/llvm-project/clang/lib/Sema/SemaModule.cpp
/var/lib/buildkite-agent/builds/llvm-project/clang/lib/Sema/SemaModule.cpp:722:53:
error: member reference type 'clang::Module *' is a pointer; did you mean to
use '->'?
assert(!ModuleScopes.empty() && getCurrentModule().isGlobalModule() &&
~~~~~~~~~~~~~~~~~~^
->
/usr/include/assert.h:93:27: note: expanded from macro 'assert'
(static_cast <bool> (expr) \
^~~~
1 error generated.
```
================
Comment at: clang/test/CXX/module/module.linkage_specification/p1.cpp:1
+// RUN: %clang_cc1 -std=c++2a %s -verify
+// expected-no-diagnostics
----------------
Might as well switch all these to use `-std=c++20`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110215/new/
https://reviews.llvm.org/D110215
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
