Index: include/clang/AST/DeclBase.h =================================================================== --- include/clang/AST/DeclBase.h +++ include/clang/AST/DeclBase.h @@ -20,6 +20,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/Specifiers.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/GraphTraits.h" #include "llvm/ADT/PointerIntPair.h" #include "llvm/ADT/PointerUnion.h" #include "llvm/ADT/iterator.h" @@ -2068,6 +2069,28 @@ } }; +template<> +struct GraphTraits { + using NodeRef = const ::clang::Decl *; + using ChildIteratorType = ::clang::DeclContext::decl_iterator; + + static NodeRef getEntryNode(const ::clang::DeclContext *DC) { + return cast<::clang::Decl>(DC); + } + + static ChildIteratorType child_begin(NodeRef D) { + if (auto *DC = dyn_cast<::clang::DeclContext>(D)) + return DC->decls_begin(); + return ChildIteratorType(); + } + + static ChildIteratorType child_end(NodeRef D) { + if (auto *DC = dyn_cast<::clang::DeclContext>(D)) + return DC->decls_end(); + return ChildIteratorType(); + } +}; + } // namespace llvm #endif // LLVM_CLANG_AST_DECLBASE_H Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -9097,6 +9097,14 @@ "export declaration can only be used within a module interface unit after " "the module declaration">; +def err_export_anonymous : Error< + "%select{unnamed namespaces|anonymous unions at namespace or global scope}0" + " cannot be exported">; +def err_export_internal_name : Error< + "%0 cannot be exported since it has internal linkage">; +def note_internal_from_unnamed_namespace : Note< + "internal linkage is due to inclusion in this unnamed namespace">; + def ext_equivalent_internal_linkage_decl_in_modules : ExtWarn< "ambiguous use of internal linkage declaration %0 defined in multiple modules">, InGroup>; Index: lib/Sema/SemaDecl.cpp =================================================================== --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -42,6 +42,7 @@ #include "clang/Sema/ScopeInfo.h" #include "clang/Sema/SemaInternal.h" #include "clang/Sema/Template.h" +#include "llvm/ADT/DepthFirstIterator.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/Triple.h" #include @@ -16992,6 +16993,89 @@ VisibleModules.setVisible(Mod, Loc); } +static const NamespaceDecl *findNearestEnclosingAnonymousNamespace(const Decl *D) { + for (auto *DC = D->getDeclContext(); DC; DC = DC->getParent()) { + if (auto *Namespace = dyn_cast(DC)) { + if (Namespace->isAnonymousNamespace()) + return Namespace; + } + } + return nullptr; +} + +/// Check whether it's okay for the given namespace-scope decl to be exported. +/// An appropriate diagnostic is emitted in the erroneous cases. +static bool validateExportedDecl(Sema &SemaRef, const Decl *D) { + assert(D); + assert(D->getDeclContext()->getRedeclContext()->isFileContext() && + "Declaration isn't at namespace-scope"); + + // Anonymous namespaces have internal linkage; they can never be exported. + if (auto *Namespace = dyn_cast(D)) { + if (Namespace->isAnonymousNamespace()) { + SemaRef.Diag(Namespace->getLocStart(), diag::err_export_anonymous) + << 0/* namespace */; + return false; + } + } + + // Namespace-scope anonymous unions must be declared as static, and so they + // can't be exported. This is special-cased because the variable name will be + // empty, which makes for a confusing diagnostic. + if (auto *Var = dyn_cast(D)) { + QualType Ty = Var->getType(); + auto *Record = Ty.isNull() + ? nullptr + : dyn_cast_or_null(Ty->getAsTagDecl()); + if (Record && Record->isAnonymousStructOrUnion()) { + // Anonymous structs are also treated as erroneous but they're left + // undiagnosed; they aren't valid at namespace-scope anyway so an + // additional diagnostic would be pointless. + if (Record->isUnion()) { + SemaRef.Diag(Record->getLocStart(), diag::err_export_anonymous) + << 1/* union */; + } + return false; + } + } + + // Ignore the injected fields from an anonymous union; we've already diagnosed + // the union variable itself. + if (isa(D)) + return false; + + // Diagnose any other named declaration with internal linkage. + // Note that UsingDirectiveDecl inherits from NamedDecl even though they + // aren't actually named, so ignore those. + if (auto *ND = dyn_cast(D)) { + if (!isa(ND) && + (!isa(ND) || cast(ND)->hasNameForLinkage()) && + ND->getFormalLinkage() == InternalLinkage) { + SemaRef.Diag(ND->getLocation(), + diag::err_export_internal_name) << ND; + + const bool IsExplicitlyStatic = [ND] { + if (auto *Var = dyn_cast(ND)) + return Var->getStorageClass() == SC_Static; + if (auto *Func = dyn_cast(ND)) + return Func->getStorageClass() == SC_Static; + return false; + }(); + + if (IsExplicitlyStatic) { + // FIXME: Add a fixit hint to remove the static keyword. + } else if (auto *AnonNS = findNearestEnclosingAnonymousNamespace(ND)) { + SemaRef.Diag(AnonNS->getLocStart(), + diag::note_internal_from_unnamed_namespace); + } + + return false; + } + } + + return true; +} + /// We have parsed the start of an export declaration, including the '{' /// (if present). Decl *Sema::ActOnStartExportDecl(Scope *S, SourceLocation ExportLoc, @@ -17024,8 +17108,23 @@ if (RBraceLoc.isValid()) ED->setRBraceLoc(RBraceLoc); - // FIXME: Diagnose export of internal-linkage declaration (including - // anonymous namespace). + // Traverse the subtree of exported nodes to ensure that they all have external + // linkage, and emit an appropriate diagnostic if that isn't the case. + auto It = llvm::df_iterator::begin(ED), + End = llvm::df_iterator::end(ED); + ++It; // Skip the ExportDecl + for (bool VisitChildren; It != End; VisitChildren ? ++It : It.skipChildren()) { + const Decl *D = *It; + if (validateExportedDecl(*this, D)) { + // We only need to check namespace-scope declarations. + VisitChildren = isa(D) && + cast(D)->getRedeclContext()->isFileContext(); + } else { + // Skip the children of erroneously exported nodes to avoid "diagnostic + // bombing" when a namespace with internal linkage is exported. + VisitChildren = false; + } + } PopDeclContext(); return D; Index: test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp =================================================================== --- test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp +++ test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp @@ -3,12 +3,10 @@ // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global // CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global -// CHECK-DAG: @_ZW6ModuleE19static_var_exported = available_externally {{(dso_local )?}}global i32 0, // CHECK-DAG: @const_var_exported = available_externally {{(dso_local )?}}constant i32 3, // // CHECK-DAG: @_ZW6ModuleE25extern_var_module_linkage = external {{(dso_local )?}}global // CHECK-DAG: @_ZW6ModuleE25inline_var_module_linkage = linkonce_odr {{(dso_local )?}}global -// CHECK-DAG: @_ZW6ModuleE25static_var_module_linkage = available_externally {{(dso_local )?}}global i32 0, // CHECK-DAG: @_ZW6ModuleE24const_var_module_linkage = available_externally {{(dso_local )?}}constant i32 3, module Module; @@ -21,7 +19,6 @@ (void)&extern_var_exported; (void)&inline_var_exported; - (void)&static_var_exported; // FIXME: Should not be exported. (void)&const_var_exported; // FIXME: This symbol should not be visible here. @@ -36,6 +33,5 @@ (void)&extern_var_module_linkage; (void)&inline_var_module_linkage; - (void)&static_var_module_linkage; // FIXME: Should not be visible here. (void)&const_var_module_linkage; } Index: test/CXX/modules-ts/basic/basic.def.odr/p4/module.cppm =================================================================== --- test/CXX/modules-ts/basic/basic.def.odr/p4/module.cppm +++ test/CXX/modules-ts/basic/basic.def.odr/p4/module.cppm @@ -11,7 +11,6 @@ // can discard this global and its initializer (if any), and other TUs are not // permitted to run the initializer for this variable. // CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global -// CHECK-DAG: @_ZW6ModuleE19static_var_exported = {{(dso_local )?}}global // CHECK-DAG: @const_var_exported = {{(dso_local )?}}constant // // CHECK-DAG: @_ZW6ModuleE25extern_var_module_linkage = external {{(dso_local )?}}global @@ -58,32 +57,20 @@ export module Module; export { - // FIXME: These should be ill-formed: you can't export an internal linkage - // symbol, per [dcl.module.interface]p2. - // CHECK: define {{(dso_local )?}}void {{.*}}@_ZW6ModuleE22unused_static_exportedv - static void unused_static_exported() {} - // CHECK: define {{(dso_local )?}}void {{.*}}@_ZW6ModuleE20used_static_exportedv - static void used_static_exported() {} - inline void unused_inline_exported() {} inline void used_inline_exported() {} extern int extern_var_exported; inline int inline_var_exported; - // FIXME: This should be ill-formed: you can't export an internal linkage - // symbol. - static int static_var_exported; const int const_var_exported = 3; // CHECK: define {{(dso_local )?}}void {{.*}}@_Z18noninline_exportedv void noninline_exported() { - used_static_exported(); // CHECK: define linkonce_odr {{.*}}@_Z20used_inline_exportedv used_inline_exported(); (void)&extern_var_exported; (void)&inline_var_exported; - (void)&static_var_exported; (void)&const_var_exported; } } Index: test/CXX/modules-ts/basic/basic.def.odr/p4/user.cpp =================================================================== --- test/CXX/modules-ts/basic/basic.def.odr/p4/user.cpp +++ test/CXX/modules-ts/basic/basic.def.odr/p4/user.cpp @@ -3,7 +3,6 @@ // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global // CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global -// CHECK-DAG: @_ZW6ModuleE19static_var_exported = available_externally {{(dso_local )?}}global i32 0 // CHECK-DAG: @const_var_exported = available_externally {{(dso_local )?}}constant i32 3 import Module; @@ -16,7 +15,6 @@ (void)&extern_var_exported; (void)&inline_var_exported; - (void)&static_var_exported; (void)&const_var_exported; // Module-linkage declarations are not visible here. Index: test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p2.cpp =================================================================== --- test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p2.cpp +++ test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p2.cpp @@ -0,0 +1,26 @@ +// RUN: %clang_cc1 -fmodules-ts %s -emit-module-interface -verify -o /dev/null + +export module A; + +export namespace { } // expected-error {{unnamed namespace}} + +export static int n = 5; // expected-error {{internal linkage}} + +namespace { // expected-note 5 {{in this}} + export { + int a = 1; // expected-error {{internal linkage}} + void f() { } // expected-error {{internal linkage}} + class B { }; // expected-error {{internal linkage}} + struct { } x; // expected-error {{internal linkage}} + + extern "C++" void g() { } // expected-error {{internal linkage}} + } +} + +export namespace a { + namespace b { + namespace c { + static int i = 3; // expected-error {{internal linkage}} + } + } +} Index: test/SemaCXX/anonymous-union-export.cpp =================================================================== --- test/SemaCXX/anonymous-union-export.cpp +++ test/SemaCXX/anonymous-union-export.cpp @@ -1,6 +1,14 @@ // RUN: %clang_cc1 -std=c++17 -fmodules-ts -emit-obj -verify -o %t.pcm %s export module M; + export { - union { bool a; }; // expected-error{{anonymous unions at namespace or global scope must be declared 'static'}} + union { bool a; }; // expected-error{{anonymous unions at namespace or global scope must be declared 'static'}} \ + // expected-error{{anonymous unions at namespace or global scope cannot be exported}} + + static union { bool a; }; // expected-error{{anonymous unions at namespace or global scope cannot be exported}} +} + +namespace { + export union { bool a; }; // expected-error{{anonymous unions at namespace or global scope cannot be exported}} }