hamzasood updated this revision to Diff 156681.
hamzasood added a comment.
- Fixed the premature loop termination issue pointed out by @rsmith (and added
a test case for it).
- Fixed a few small issues regarding diagnosing exported enum/struct/union
declarations without a name (especially anonymous unions at namespace-scope).
- Factored out the loop body into a separate function to keep things tidy.
https://reviews.llvm.org/D41566
Files:
include/clang/AST/DeclBase.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaDecl.cpp
test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp
test/CXX/modules-ts/basic/basic.def.odr/p4/module.cppm
test/CXX/modules-ts/basic/basic.def.odr/p4/user.cpp
test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.interface/p2.cpp
test/SemaCXX/anonymous-union-export.cpp
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}}
}
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/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/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/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: 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 <algorithm>
@@ -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<NamespaceDecl>(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<NamespaceDecl>(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<VarDecl>(D)) {
+ QualType Ty = Var->getType();
+ auto *Record = Ty.isNull()
+ ? nullptr
+ : dyn_cast_or_null<RecordDecl>(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<IndirectFieldDecl>(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<NamedDecl>(D)) {
+ if (!isa<UsingDirectiveDecl>(ND) &&
+ (!isa<TagDecl>(ND) || cast<TagDecl>(ND)->hasNameForLinkage()) &&
+ ND->getFormalLinkage() == InternalLinkage) {
+ SemaRef.Diag(ND->getLocation(),
+ diag::err_export_internal_name) << ND;
+
+ const bool IsExplicitlyStatic = [ND] {
+ if (auto *Var = dyn_cast<VarDecl>(ND))
+ return Var->getStorageClass() == SC_Static;
+ if (auto *Func = dyn_cast<FunctionDecl>(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<const DeclContext *>::begin(ED),
+ End = llvm::df_iterator<const DeclContext *>::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<DeclContext>(D) &&
+ cast<DeclContext>(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: 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<DiagGroup<"modules-ambiguous-internal-linkage">>;
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<const ::clang::DeclContext *> {
+ 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
