[PATCH] D41566: [Modules TS] Diagnose exported internal linkage declarations

[Hamza Sood via Phabricator via cfe-commits]

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)_var_exported;
   (void)_var_exported;
-  (void)_var_exported;
   (void)_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 

[PATCH] D41566: [Modules TS] Diagnose exported internal linkage declarations

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: include/clang/AST/DeclBase.h:2055-2075
+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);

Neat :)



Comment at: lib/Sema/SemaDecl.cpp:16354
+if (auto *DC = dyn_cast(*It))
+  VisitChildren = DC->isLookupContext();
+

This will prematurely stop traversal if we reach a `LinkageSpecDecl`. I think 
we should instead stop if we reach a declaration for which 
`!DC->getRedeclContext()->isFileContext()` (that is, we should check 
namespace-scope declarations only).


https://reviews.llvm.org/D41566



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41566: [Modules TS] Diagnose exported internal linkage declarations

[Hamza Sood via Phabricator via cfe-commits]

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)_var_exported;
   (void)_var_exported;
-  (void)_var_exported;
   (void)_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)_var_exported;
 (void)_var_exported;
-(void)_var_exported;
 (void)_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)_var_exported;
   (void)_var_exported;
-  (void)_var_exported; // FIXME: Should not be exported.
   

[PATCH] D41566: [Modules TS] Diagnose exported internal linkage declarations

[Hamza Sood via Phabricator via cfe-commits]

hamzasood created this revision.
hamzasood added reviewers: rsmith, bruno, boris.

Diagnose attempts to export declarations with internal linkage, as mentioned in 
`[dcl.module.interface]p2` in the Modules TS.

I would've liked to add a FixIt hint to remove the static keyword if present, 
but I couldn't work out how to get the `SourceRange` that covers it (it's 
accessible from the `Declarator`, but seemingly not after the actual node has 
been created).
I've left a fixme comment in there in case anyone else can figure it out.


https://reviews.llvm.org/D41566

Files:
  include/clang/AST/DeclBase.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/DeclBase.cpp
  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)_var_exported;
   (void)_var_exported;
-  (void)_var_exported;
   (void)_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)_var_exported;
 (void)_var_exported;
-(void)_var_exported;
 (void)_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 =