[C++20][Modules] Do not allow non-inline external definitions in header units.

[module.import/6] last sentence:
A header unit shall not contain a definition of a non-inline function or
variable whose name has external linkage.

Differential Revision: https://reviews.llvm.org/D140261

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 91cb904..b623d26 100644 (file)
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -187,6 +187,11 @@ code bases.
 - ``-p`` is rejected for all targets which are not AIX or OpenBSD. ``-p`` led
   to an ``-Wunused-command-line-argument`` warning in previous releases.
 
+- Clang now diagnoses non-inline externally-visible definitions in C++
+  standard header units as per ``[module.import/6]``.  Previously, in Clang-15,
+  these definitions were allowed.  Note that such definitions are ODR
+  violations if the header is included more than once.
+
 What's New in Clang |release|?
 ==============================
 Some of the major new features and improvements to Clang are listed

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index d1bffc7..7154a8b 100644 (file)
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11236,6 +11236,8 @@ def note_not_module_interface_add_export : Note<
   "add 'export' here if this is intended to be a module interface unit">;
 def err_invalid_module_name : Error<
   "%0 is %select{an invalid|a reserved}1 name for a module">;
+def err_extern_def_in_header_unit : Error<
+  "non-inline external definitions are not permitted in C++ header units">;
 
 def ext_equivalent_internal_linkage_decl_in_modules : ExtWarn<
   "ambiguous use of internal linkage declaration %0 defined in multiple modules">,

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1cbbcfd..4714b8b 100644 (file)
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2325,6 +2325,12 @@ public:
     return ModuleScopes.empty() ? false : ModuleScopes.back().ModuleInterface;
   }
 
+  /// Is the module scope we are in a C++ Header Unit?
+  bool currentModuleIsHeaderUnit() const {
+    return ModuleScopes.empty() ? false
+                                : ModuleScopes.back().Module->isHeaderUnit();
+  }
+
   /// Get the module owning an entity.
   Module *getOwningModule(const Decl *Entity) {
     return Entity->getOwningModule();

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index a1bfb57..d25f513 100644 (file)
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13071,6 +13071,15 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
       VDecl->setInvalidDecl();
   }
 
+  // C++ [module.import/6] external definitions are not permitted in header
+  // units.
+  if (getLangOpts().CPlusPlusModules && currentModuleIsHeaderUnit() &&
+      VDecl->getFormalLinkage() == Linkage::ExternalLinkage &&
+      !VDecl->isInline()) {
+    Diag(VDecl->getLocation(), diag::err_extern_def_in_header_unit);
+    VDecl->setInvalidDecl();
+  }
+
   // If adding the initializer will turn this declaration into a definition,
   // and we already have a definition for this variable, diagnose or otherwise
   // handle the situation.
@@ -15224,6 +15233,14 @@ Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D,
     }
   }
 
+  // C++ [module.import/6] external definitions are not permitted in header
+  // units.
+  if (getLangOpts().CPlusPlusModules && currentModuleIsHeaderUnit() &&
+      FD->getFormalLinkage() == Linkage::ExternalLinkage && !FD->isInlined()) {
+    Diag(FD->getLocation(), diag::err_extern_def_in_header_unit);
+    FD->setInvalidDecl();
+  }
+
   // Ensure that the function's exception specification is instantiated.
   if (const FunctionProtoType *FPT = FD->getType()->getAs<FunctionProtoType>())
     ResolveExceptionSpec(D->getLocation(), FPT);

diff --git a/clang/test/CXX/module/module.import/p6.cpp b/clang/test/CXX/module/module.import/p6.cpp
new file mode 100644 (file)
index 0000000..25c1950
--- /dev/null
+++ b/clang/test/CXX/module/module.import/p6.cpp
@@ -0,0 +1,24 @@
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -x c++-header %t/bad-header-unit.h \
+// RUN:  -emit-header-unit -o %t/bad-header-unit.pcm -verify
+
+//--- bad-header-unit.h
+
+inline int ok_foo () { return 0;}
+
+static int ok_bar ();
+
+int ok_decl ();
+
+int bad_def () { return 2;}  // expected-error {{non-inline external definitions are not permitted in C++ header units}}
+
+inline int ok_inline_var = 1;
+
+static int ok_static_var;
+
+int ok_var_decl;
+
+int bad_var_definition = 3;  // expected-error {{non-inline external definitions are not permitted in C++ header units}}
+

diff --git a/clang/test/CodeGenCXX/module-initializer-header.cppm b/clang/test/CodeGenCXX/module-initializer-header.cppm
index 253f096..5cd9352 100644 (file)
--- a/clang/test/CodeGenCXX/module-initializer-header.cppm
+++ b/clang/test/CodeGenCXX/module-initializer-header.cppm
@@ -8,24 +8,24 @@
 //
 //--- header.h
 int foo();
-int i = foo();
+static int i = foo();
 
 //--- M.cppm
 module;
 import "header.h";
 export module M;
 
-// CHECK: @i = {{.*}}global i32 0
+// CHECK: @_ZL1i = {{.*}}global i32 0
 // CHECK: void @__cxx_global_var_init()
 // CHECK-NEXT: entry:
 // CHECK-NEXT:  %call = call noundef{{.*}} i32 @_Z3foov()
-// CHECK-NEXT:  store i32 %call, ptr @i  
+// CHECK-NEXT:  store i32 %call, ptr @_ZL1i
 
 //--- Use.cpp
 import "header.h";
 
-// CHECK: @i = {{.*}}global i32 0
+// CHECK: @_ZL1i = {{.*}}global i32 0
 // CHECK: void @__cxx_global_var_init()
 // CHECK-NEXT: entry:
 // CHECK-NEXT:  %call = call noundef{{.*}} i32 @_Z3foov()
-// CHECK-NEXT:  store i32 %call, ptr @i  
+// CHECK-NEXT:  store i32 %call, ptr @_ZL1i