hamzasood updated this revision to Diff 128302.
hamzasood added a comment.

I've removed the `isExported` fix for namespaces as it's somewhat unrelated to 
this patch.
I'll do a separate patch for fixing namespaces (which will include the stuff 
removed from here and a bit more)


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

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,23 @@
+// RUN: %clang_cc1 -fmodules-ts %s -emit-module-interface -verify
+
+export module A;
+
+export namespace { } // expected-error {{unnamed namespace}}
+
+export static int n = 5; // expected-error {{internal linkage}}
+
+namespace { // expected-note 3{{in this}}
+  export {
+    int a = 1;   // expected-error {{internal linkage}}
+    void f() { } // expected-error {{internal linkage}}
+    class B { }; // 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 global
 // CHECK-DAG: @inline_var_exported = linkonce_odr global
-// CHECK-DAG: @_ZW6ModuleE19static_var_exported = available_externally global i32 0
 // CHECK-DAG: @const_var_exported = available_externally 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 global
-// CHECK-DAG: @_ZW6ModuleE19static_var_exported = global
 // CHECK-DAG: @const_var_exported = constant
 //
 // CHECK-DAG: @_ZW6ModuleE25extern_var_module_linkage = external 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 void {{.*}}@_ZW6ModuleE22unused_static_exportedv
-  static void unused_static_exported() {}
-  // CHECK: define 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 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 global
 // CHECK-DAG: @inline_var_exported = linkonce_odr global
-// CHECK-DAG: @_ZW6ModuleE19static_var_exported = available_externally global i32 0,
 // CHECK-DAG: @const_var_exported = available_externally constant i32 3,
 //
 // CHECK-DAG: @_ZW6ModuleE25extern_var_module_linkage = external global
 // CHECK-DAG: @_ZW6ModuleE25inline_var_module_linkage = linkonce_odr global
-// CHECK-DAG: @_ZW6ModuleE25static_var_module_linkage = available_externally global i32 0,
 // CHECK-DAG: @_ZW6ModuleE24const_var_module_linkage = available_externally 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>
@@ -16298,6 +16299,16 @@
   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;
+}
+
 /// We have parsed the start of an export declaration, including the '{'
 /// (if present).
 Decl *Sema::ActOnStartExportDecl(Scope *S, SourceLocation ExportLoc,
@@ -16330,8 +16341,50 @@
   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. We skip
+  // the children of erroneously exported nodes to avoid "diagnostic bombing"
+  // when a namespace with internal linkage is exported.
+  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()) {
+    VisitChildren = false;
+    if (auto *DC = dyn_cast<DeclContext>(*It))
+      VisitChildren = DC->isLookupContext();
+
+    // We're checking linkage, so ignore declarations that don't have names.
+    // Note that UsingDirectiveDecl inherits from NamedDecl even though they
+    // aren't named, so skip those too.
+    auto *ND = dyn_cast<NamedDecl>(*It);
+    if (!ND || isa<UsingDirectiveDecl>(ND))
+      continue;
+
+    if (isa<NamespaceDecl>(ND) &&
+        cast<NamespaceDecl>(ND)->isAnonymousNamespace()) {
+      // Anonymous namespaces always have internal linkage.
+      // So they can never be exported.
+      Diag(ND->getLocStart(), diag::err_export_unnamed_namespace);
+      VisitChildren = false;
+    } else if (ND->getFormalLinkage() == InternalLinkage) {
+      Diag(ND->getLocation(), diag::err_export_internal_name) << ND;
+
+      bool IsExplicitlyStatic = false;
+      if (auto *Var = dyn_cast<VarDecl>(ND))
+        IsExplicitlyStatic = Var->getStorageClass() == SC_Static;
+      else if (auto *Func = dyn_cast<FunctionDecl>(ND))
+        IsExplicitlyStatic = Func->getStorageClass() == SC_Static;
+
+      if (IsExplicitlyStatic) {
+        // FIXME: Add a fixit hint to remove the static keyword.
+      } else if (auto *AnonNS = findNearestEnclosingAnonymousNamespace(ND)) {
+        Diag(AnonNS->getLocStart(),
+              diag::note_internal_from_unnamed_namespace);
+      }
+
+      VisitChildren = false;
+    }
+  }
 
   PopDeclContext();
   return D;
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -9065,6 +9065,13 @@
   "export declaration can only be used within a module interface unit after "
   "the module declaration">;
 
+def err_export_unnamed_namespace : Error<
+  "an unnamed namespace 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
@@ -21,6 +21,7 @@
 #include "clang/Basic/Specifiers.h"
 #include "clang/Basic/VersionTuple.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"
@@ -2051,6 +2052,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

Reply via email to