Skip to content

Commit

Permalink
[C++20][Modules] Do not allow non-inline external definitions in head…
Browse files Browse the repository at this point in the history
…er 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
  • Loading branch information
iains committed Jan 8, 2023
1 parent 25acbf3 commit 335668b
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 5 deletions.
5 changes: 5 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -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">,
Expand Down
6 changes: 6 additions & 0 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -2325,6 +2325,12 @@ class Sema final {
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();
Expand Down
17 changes: 17 additions & 0 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down
24 changes: 24 additions & 0 deletions clang/test/CXX/module/module.import/p6.cpp
Original file line number Diff line number Diff line change
@@ -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}}

10 changes: 5 additions & 5 deletions clang/test/CodeGenCXX/module-initializer-header.cppm
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 335668b

Please sign in to comment.