[C++2a] [Module] Support extern C/C++ semantics

[ChuanqiXu]

Summary

According to [[ https://eel.is/c++draft/module.unit#7.2.3 | [module.unit]p7.2.3]], a declaration within a linkage-specification should be attached to the global module.
This let user to forward declare types across modules. Here is an example: https://godbolt.org/z/vEWjdrdq9

Test Plan: check-all, https://godbolt.org/z/vEWjdrdq9

[ChuanqiXu]

Add test.

[ChuanqiXu]

Update test.

[mizvekov]

Hello.

Just some minor nibbles, as I am a bit out of the loop on modules.

clang/include/clang/Basic/DiagnosticSemaKinds.td

10842–10845 ↗

(On Diff #375767)

def **err_module_language_linkage_no_global** : Error <
  "The declaration %0**, which** appears within a linkage-specification**, is not "
  "attached to **the** global module.">;

My two cents since I think the last sentence did not seem to explain the problem very well.

clang/lib/Sema/SemaDecl.cpp

9353 ↗

(On Diff #375767)

16437 ↗

(On Diff #375767)

This block is of code is identical to the one above. Can you put this into a function?

clang/lib/Sema/SemaModule.cpp

71

clang/test/CXX/module/module.linkage_specification/Inputs/h1.h

13

Please if possible make your editor set new lines at EOF.

[aaron.ballman]

Adding some more reviewers and subscribing the mailing lists.

clang/include/clang/Basic/DiagnosticSemaKinds.td

10842 ↗

(On Diff #375767)

typo, should be: err_module_language_linkage_no_global

10842–10845 ↗

(On Diff #375767)

Diagnostics in Clang are intentionally ungrammatical, so they should not have leading capitalization or most terminating punctuation. I agree that the diagnostic wording here doesn't really explain what's going wrong.

clang/lib/Sema/SemaDecl.cpp

9351 ↗

(On Diff #375767)

I'm a bit confused here. [module.unit]p7 is describing what module a declaration attached to under which circumstances. I don't see a constraint there which should result in a diagnostic. My reading of https://eel.is/c++draft/module.unit#6 is that the global module always exists, so we should be able to attach declarations to it at any point. Am I misunderstanding?

clang/test/CXX/module/module.linkage_specification/Inputs/h1.h

13

+1 -- please add a newline to the end of each of these files that.

[ChuanqiXu]

Address the comments.

[ChuanqiXu]

clang/lib/Sema/SemaDecl.cpp

9351 ↗

(On Diff #375767)

Yeah, after looking into this, I think you are right. We should create the global module when we need it but failed to find it.

[ChuanqiXu]

Format.

[rsmith]

Instead of moving functions and classes/enums into the global module after creating them, it would be better to push a module scope for the global module when entering a C or C++ language linkage specification and pop the module scope when leaving the linkage specification. There are lots of other kinds of declaration that can appear inside a linkage specification (for example, variables), and doing it that way would properly handle all of them, not only functions and classes.

clang/include/clang/Sema/Sema.h

2198

I think this should live on the ModuleScope instead of being separate. If we process multiple modules in a single compilation, each needs to have its own global module fragment so that we can restrict the visibility of the global module fragment to the module unit in which it was declared.

[ChuanqiXu]

Address the comments from @rsmith. It looks much better now.

[ChuanqiXu]

@rsmith @aaron.ballman gentle ping~

[ChuanqiXu]

@rsmith gentle ping~

[aaron.ballman]

clang/lib/Sema/SemaDeclCXX.cpp

16155

Module * instead of auto * (the type is not spelled out in the initialization).

clang/lib/Sema/SemaModule.cpp

71–72

Module *

707–708

Please spell these out as well.

712–715

Can we use emplace_back() and construct in place rather than constructing piecemeal?

722–723

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.

clang/test/CXX/module/module.linkage_specification/p1.cpp

2

Might as well switch all these to use -std=c++20.

[ChuanqiXu]

Address comments.

[ChuanqiXu]

clang/lib/Sema/SemaModule.cpp

722–723

Oh, my bad. I forgot to run tests under debug locally.

[ChuanqiXu]

@rsmith @aaron.ballman gentle ping~

[ChuanqiXu]

@rsmith ping~

[ChuanqiXu]

Change includes:

• Add parent for newly created global module fragment.
• Add a test.
• Rename test directory. Suddenly I found that the names of the tests in CXX directory are corresponds to the standard one by one. So my name before is not good.

@aaron.ballman @urnathan It looks like @rsmith is busy now. Could you please help to review this one? Many Thanks.

[ChuanqiXu]

Format codes.

[aaron.ballman]

In D110215#3139589, @ChuanqiXu wrote:

Change includes:

• Add parent for newly created global module fragment.
• Add a test.
• Rename test directory. Suddenly I found that the names of the tests in CXX directory are corresponds to the standard one by one. So my name before is not good.

@aaron.ballman @urnathan It looks like @rsmith is busy now. Could you please help to review this one? Many Thanks.

@ChuanqiXu -- I'm happy to take a look (all of your modules patches are still on my review queue), but I've been in WG14 meetings all week this week and will be on vacation all week next week. Given the complexity and importance of the patches, it may be a bit before I can give this a thorough review. (If you get approval from other trusted reviewers before I get to anything, please don't let me hold any of the patches up!)

[ChuanqiXu]

In D110215#3139809, @aaron.ballman wrote:

In D110215#3139589, @ChuanqiXu wrote:

Change includes:

• Add parent for newly created global module fragment.
• Add a test.
• Rename test directory. Suddenly I found that the names of the tests in CXX directory are corresponds to the standard one by one. So my name before is not good.

@aaron.ballman @urnathan It looks like @rsmith is busy now. Could you please help to review this one? Many Thanks.

@ChuanqiXu -- I'm happy to take a look (all of your modules patches are still on my review queue), but I've been in WG14 meetings all week this week and will be on vacation all week next week. Given the complexity and importance of the patches, it may be a bit before I can give this a thorough review. (If you get approval from other trusted reviewers before I get to anything, please don't let me hold any of the patches up!)

Got it. Many thanks! It is great enough to know the review process is going on. I completely understand that we should be very careful about these patches.

[ChuanqiXu]

@aaron.ballman @urnathan @rsmith ping. Might you take a look?

[rsmith]

Thanks, this looks great. A couple of minor suggestions.

clang/include/clang/Lex/ModuleMap.h

542–543 ↗

(On Diff #388151)

We generally refer to sections of the C++ standard by name not by URL.

clang/lib/Sema/SemaDeclCXX.cpp

16172

I think this should be ModulePrivate -- such a global module fragment should not be visible to importers of this module. (I think it probably doesn't matter in practice since visibility is generally monotonically increasing while compiling a module unit, but ModulePrivate seems more correct in principle.)

[ChuanqiXu]

Address comments.

[vitalybuka]

llvm-project/clang/lib/Sema/SemaModule.cpp:715:70: warning: missing field 'OuterVisibleModules' initializer [-Wmissing-field-initializers]
                                                                    ^

[ChuanqiXu]

In D110215#3178575, @vitalybuka wrote:

llvm-project/clang/lib/Sema/SemaModule.cpp:715:70: warning: missing field 'OuterVisibleModules' initializer [-Wmissing-field-initializers]
                                                                    ^

I had tried to fix this in a NFC commit: 4168efe1b. Hope this works.
(My compiler locally didn't warn for it... I need to update it...)

clang/include/clang/Basic/Module.h

public:
   /// eventually be exposed, for use in "private" modules.
   std::string ExportAsModule;
 
   /// Does this Module scope describe part of the purview of a named C++ module?
   bool isModulePurview() const {
     return Kind == ModuleInterfaceUnit || Kind == PrivateModuleFragment;
   }
 
+  /// Does this Module scope describe a fragment of the global module within
+  /// some C++ module.
+  bool isGlobalModule() const { return Kind == GlobalModuleFragment; }
+
 private:
   /// The submodules of this module, indexed by name.
   std::vector<Module *> SubModules;
 
   /// A mapping from the submodule name to the index into the
   /// \c SubModules vector at which that submodule resides.
   llvm::StringMap<unsigned> SubModuleIndex;
 

clang/include/clang/Sema/Sema.h

struct ModuleScope {
     SourceLocation BeginLoc;
     clang::Module *Module = nullptr;
     bool ModuleInterface = false;
     bool ImplicitGlobalModuleFragment = false;
     VisibleModuleSet OuterVisibleModules;
   };
   /// The modules we're currently parsing.
   llvm::SmallVector<ModuleScope, 16> ModuleScopes;
 
   /// Namespace definitions that we will export when they finish.
   llvm::SmallPtrSet<const NamespaceDecl*, 8> DeferredExportedNamespaces;
 
   /// Get the module whose scope we are currently within.
   Module *getCurrentModule() const {
     return ModuleScopes.empty() ? nullptr : ModuleScopes.back().Module;
   }
 
+  /// Enter the scope of the global module.
+  Module *PushGlobalModuleFragment(SourceLocation BeginLoc, bool IsImplicit);
+  /// Leave the scope of the global module.
+  void PopGlobalModuleFragment();
+
   VisibleModuleSet VisibleModules;
 
 public:
   /// Get the module owning an entity.
   Module *getOwningModule(const Decl *Entity) {
     return Entity->getOwningModule();
   }
 

clang/lib/Sema/SemaDeclCXX.cpp

     return nullptr;
   }
 
   // FIXME: Add all the various semantics of linkage specifications
 
   LinkageSpecDecl *D = LinkageSpecDecl::Create(Context, CurContext, ExternLoc,
                                                LangStr->getExprLoc(), Language,
                                                LBraceLoc.isValid());
+
+  /// C++ [module.unit]p7.2.3
+  /// - Otherwise, if the declaration
+  ///   - ...
+  ///   - ...
+  ///   - appears within a linkage-specification,
+  ///   it is attached to the global module.
+  if (getLangOpts().CPlusPlusModules) {
+    Module *GlobalModule =
+        PushGlobalModuleFragment(ExternLoc, /*IsImplicit=*/true);
+    D->setModuleOwnershipKind(Decl::ModuleOwnershipKind::Visible);
+    D->setLocalOwningModule(GlobalModule);
+  }
+
   CurContext->addDecl(D);
   PushDeclContext(S, D);
   return D;
 }
 
 /// ActOnFinishLinkageSpecification - Complete the definition of
 /// the C++ linkage specification LinkageSpec. If RBraceLoc is
 /// valid, it's the position of the closing '}' brace in a linkage
 /// specification that uses braces.
 Decl *Sema::ActOnFinishLinkageSpecification(Scope *S,
                                             Decl *LinkageSpec,
                                             SourceLocation RBraceLoc) {
   if (RBraceLoc.isValid()) {
     LinkageSpecDecl* LSDecl = cast<LinkageSpecDecl>(LinkageSpec);
     LSDecl->setRBraceLoc(RBraceLoc);
   }
+
+  if (getLangOpts().CPlusPlusModules)
+    PopGlobalModuleFragment();
+
   PopDeclContext();
   return LinkageSpec;
 }
 
 Decl *Sema::ActOnEmptyDeclaration(Scope *S,
                                   const ParsedAttributesView &AttrList,
                                   SourceLocation SemiLoc) {
   Decl *ED = EmptyDecl::Create(Context, CurContext, SemiLoc);

clang/lib/Sema/SemaModule.cpp

if (!ModuleScopes.empty() &&
     assert(getLangOpts().CPlusPlusModules && getLangOpts().ModulesTS &&
            "unexpectedly encountered multiple global module fragment decls");
     ModuleScopes.back().BeginLoc = ModuleLoc;
     return nullptr;
   }
 
   // We start in the global module; all those declarations are implicitly
   // 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);
+  Module *GlobalModule =
+      PushGlobalModuleFragment(ModuleLoc, /*IsImplicit=*/false);
 
   // All declarations created from now on are owned by the global module.
   auto *TU = Context.getTranslationUnitDecl();
   TU->setModuleOwnershipKind(Decl::ModuleOwnershipKind::Visible);
   TU->setLocalOwningModule(GlobalModule);
 
   // FIXME: Consider creating an explicit representation of this declaration.
   return nullptr;
for (auto *Child : ED->decls()) {
         diagExportedUnnamedDecl(*this, UnnamedDeclKind::Context, Child,
                                 BlockStart);
       }
     }
   }
 
   return D;
 }
+
+Module *Sema::PushGlobalModuleFragment(SourceLocation BeginLoc,
+                                       bool IsImplicit) {
+  ModuleMap &Map = PP.getHeaderSearchInfo().getModuleMap();
+  Module *GlobalModule = Map.createGlobalModuleFragmentForModuleUnit(BeginLoc);
+  assert(GlobalModule && "module creation should not fail");
+
+  // Enter the scope of the global module.
+  ModuleScopes.push_back({BeginLoc, GlobalModule,
+                          /*ModuleInterface=*/false,
+                          /*ImplicitGlobalModuleFragment=*/IsImplicit});
+  VisibleModules.setVisible(GlobalModule, BeginLoc);
+
+  return GlobalModule;
+}
+
+void Sema::PopGlobalModuleFragment() {
+  assert(!ModuleScopes.empty() && getCurrentModule()->isGlobalModule() &&
+         "left the wrong module scope, which is not global module fragment");
+  ModuleScopes.pop_back();
+}

clang/test/CXX/module/module.linkage_specification/Inputs/h1.h

+extern "C" void foo();
+extern "C" {
+void bar();
+int baz();
+double double_func();
+}
+
+extern "C++" {
+void bar_cpp();
+int baz_cpp();
+double double_func_cpp();
+}

clang/test/CXX/module/module.linkage_specification/Inputs/h2.h

+extern "C++" class CPP;

clang/test/CXX/module/module.linkage_specification/Inputs/h4.h

+extern "C" struct C;

clang/test/CXX/module/module.linkage_specification/Inputs/h5.h

+extern "C++" int a;

clang/test/CXX/module/module.linkage_specification/p1.cpp

+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h1.h"
+
+export module x;
+
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {
+void bar() {
+  return;
+}
+int baz() {
+  return 3;
+}
+double double_func() {
+  return 5.0;
+}
+}
+
+extern "C++" {
+void bar_cpp() {
+  return;
+}
+int baz_cpp() {
+  return 3;
+}
+double double_func_cpp() {
+  return 5.0;
+}
+}

clang/test/CXX/module/module.linkage_specification/p2.cpp

+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h2.h"
+
+export module x;
+
+extern "C++" class CPP {};

clang/test/CXX/module/module.linkage_specification/p3.cpp

+// This tests whether the global module would be created when the program don't declare it explicitly.
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+export module x;
+
+extern "C" void foo();
+extern "C++" class CPP {};

clang/test/CXX/module/module.linkage_specification/p4.cpp

+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C" struct C {
+  int a;
+  int b;
+  double d;
+};

clang/test/CXX/module/module.linkage_specification/p5.cpp

+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C++" int a = 5;

clang/test/CodeGenCXX/Inputs/module-extern-C.h

+extern "C" void foo();
+extern "C" {
+void bar();
+int baz();
+double double_func();
+}
+extern "C++" class CPP;

clang/test/CodeGenCXX/module-extern-C.cpp

+// RUN: %clang_cc1 -std=c++20 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+
+module;
+
+#include "Inputs/module-extern-C.h"
+
+export module x;
+
+// CHECK: define dso_local void @foo()
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {
+// CHECK: define dso_local void @bar()
+void bar() {
+  return;
+}
+// CHECK: define dso_local i32 @baz()
+int baz() {
+  return 3;
+}
+// CHECK: define dso_local double @double_func()
+double double_func() {
+  return 5.0;
+}
+}