[clang] 3a3af2b - [C++20] [Module] fix bug 47716 and implement [module.interface]/p6

Chuanqi Xu via cfe-commits cfe-commits at lists.llvm.org
Sun Jan 23 18:37:00 PST 2022


Author: Chuanqi Xu
Date: 2022-01-24T10:25:25+08:00
New Revision: 3a3af2bbc97e7db045eccb8683e93b9aa7ef562b

URL: https://github.com/llvm/llvm-project/commit/3a3af2bbc97e7db045eccb8683e93b9aa7ef562b
DIFF: https://github.com/llvm/llvm-project/commit/3a3af2bbc97e7db045eccb8683e93b9aa7ef562b.diff

LOG: [C++20] [Module] fix bug 47716 and implement [module.interface]/p6

This fixes bug 47716.

According to [module.interface]p2, it is meaningless to export an entity
which is not in namespace scope.
The reason why the compiler crashes is that the compiler missed
ExportDecl when the compiler traverse the subclass of DeclContext. So
here is the crash.

Also, the patch implements [module.interface]p6 in
Sema::CheckRedeclaration* functions.

Reviewed By: aaron.ballman, urnathan

Differential Revision: https://reviews.llvm.org/D112903

Added: 
    clang/test/CXX/module/module.interface/p2-2.cpp
    clang/test/CXX/module/module.interface/p6.cpp

Modified: 
    clang/include/clang/AST/DeclBase.h
    clang/include/clang/Basic/DiagnosticSemaKinds.td
    clang/include/clang/Sema/Sema.h
    clang/lib/AST/DeclBase.cpp
    clang/lib/Sema/SemaDecl.cpp
    clang/lib/Sema/SemaDeclCXX.cpp
    clang/lib/Sema/SemaTemplate.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h
index 1328d377d00fa..06d2f17d14300 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -607,6 +607,20 @@ class alignas(8) Decl {
     return getModuleOwnershipKind() == ModuleOwnershipKind::ModulePrivate;
   }
 
+  /// Whether this declaration was exported in a lexical context.
+  /// e.g.:
+  ///
+  ///   export namespace A {
+  ///      void f1();        // isInExportDeclContext() == true
+  ///   }
+  ///   void A::f1();        // isInExportDeclContext() == false
+  ///
+  ///   namespace B {
+  ///      void f2();        // isInExportDeclContext() == false
+  ///   }
+  ///   export void B::f2(); // isInExportDeclContext() == true
+  bool isInExportDeclContext() const;
+
   /// Return true if this declaration has an attribute which acts as
   /// definition of the entity, such as 'alias' or 'ifunc'.
   bool hasDefiningAttr() const;

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 88e430d8eb09f..7fccdcaa9fc6a 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7803,6 +7803,11 @@ def err_expected_class_or_namespace : Error<"%0 is not a class"
   "%select{ or namespace|, namespace, or enumeration}1">;
 def err_invalid_declarator_scope : Error<"cannot define or redeclare %0 here "
   "because namespace %1 does not enclose namespace %2">;
+def err_export_non_namespace_scope_name : Error<
+  "cannot export %0 as it is not at namespace scope">;
+def err_redeclaration_non_exported : Error <
+  "cannot export redeclaration %0 here since the previous declaration is not "
+  "exported">;
 def err_invalid_declarator_global_scope : Error<
   "definition or redeclaration of %0 cannot name the global scope">;
 def err_invalid_declarator_in_function : Error<

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index b1ef02865328f..4b609f4b1477c 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4320,6 +4320,8 @@ class Sema final {
                             bool ConsiderLinkage, bool AllowInlineNamespace);
 
   bool CheckRedeclarationModuleOwnership(NamedDecl *New, NamedDecl *Old);
+  bool CheckRedeclarationExported(NamedDecl *New, NamedDecl *Old);
+  bool CheckRedeclarationInModule(NamedDecl *New, NamedDecl *Old);
 
   void DiagnoseAmbiguousLookup(LookupResult &Result);
   //@}

diff  --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index 98a5c6b664713..9ee1cc0830867 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -995,6 +995,15 @@ bool Decl::AccessDeclContextCheck() const {
   return true;
 }
 
+bool Decl::isInExportDeclContext() const {
+  const DeclContext *DC = getLexicalDeclContext();
+
+  while (DC && !isa<ExportDecl>(DC))
+    DC = DC->getLexicalParent();
+
+  return DC && isa<ExportDecl>(DC);
+}
+
 static Decl::Kind getKind(const Decl *D) { return D->getKind(); }
 static Decl::Kind getKind(const DeclContext *DC) { return DC->getDeclKind(); }
 

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index d4ed721e0545b..a29409461f575 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -1628,6 +1628,39 @@ bool Sema::CheckRedeclarationModuleOwnership(NamedDecl *New, NamedDecl *Old) {
   return false;
 }
 
+// [module.interface]p6:
+// A redeclaration of an entity X is implicitly exported if X was introduced by
+// an exported declaration; otherwise it shall not be exported.
+bool Sema::CheckRedeclarationExported(NamedDecl *New, NamedDecl *Old) {
+  bool IsNewExported = New->isInExportDeclContext();
+  bool IsOldExported = Old->isInExportDeclContext();
+
+  // It should be irrevelant if both of them are not exported.
+  if (!IsNewExported && !IsOldExported)
+    return false;
+
+  if (IsOldExported)
+    return false;
+
+  assert(IsNewExported);
+
+  Diag(New->getLocation(), diag::err_redeclaration_non_exported) << New;
+  Diag(Old->getLocation(), diag::note_previous_declaration);
+  return true;
+}
+
+// A wrapper function for checking the semantic restrictions of
+// a redeclaration within a module.
+bool Sema::CheckRedeclarationInModule(NamedDecl *New, NamedDecl *Old) {
+  if (CheckRedeclarationModuleOwnership(New, Old))
+    return true;
+
+  if (CheckRedeclarationExported(New, Old))
+    return true;
+
+  return false;
+}
+
 static bool isUsingDecl(NamedDecl *D) {
   return isa<UsingShadowDecl>(D) ||
          isa<UnresolvedUsingTypenameDecl>(D) ||
@@ -3390,7 +3423,7 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD,
     }
   }
 
-  if (CheckRedeclarationModuleOwnership(New, Old))
+  if (CheckRedeclarationInModule(New, Old))
     return true;
 
   if (!getLangOpts().CPlusPlus) {
@@ -4269,7 +4302,7 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) {
     return New->setInvalidDecl();
   }
 
-  if (CheckRedeclarationModuleOwnership(New, Old))
+  if (CheckRedeclarationInModule(New, Old))
     return;
 
   // Variables with external linkage are analyzed in FinalizeDeclaratorGroup.
@@ -5759,7 +5792,15 @@ bool Sema::diagnoseQualifiedDeclaration(CXXScopeSpec &SS, DeclContext *DC,
     else if (isa<BlockDecl>(Cur))
       Diag(Loc, diag::err_invalid_declarator_in_block)
         << Name << SS.getRange();
-    else
+    else if (isa<ExportDecl>(Cur)) {
+      if (!isa<NamespaceDecl>(DC))
+        Diag(Loc, diag::err_export_non_namespace_scope_name)
+            << Name << SS.getRange();
+      else
+        // The cases that DC is not NamespaceDecl should be handled in
+        // CheckRedeclarationExported.
+        return false;
+    } else
       Diag(Loc, diag::err_invalid_declarator_scope)
       << Name << cast<NamedDecl>(Cur) << cast<NamedDecl>(DC) << SS.getRange();
 
@@ -16535,7 +16576,7 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
     SetMemberAccessSpecifier(New, PrevDecl, AS);
 
   if (PrevDecl)
-    CheckRedeclarationModuleOwnership(New, PrevDecl);
+    CheckRedeclarationInModule(New, PrevDecl);
 
   if (TUK == TUK_Definition && (!SkipBody || !SkipBody->ShouldSkip))
     New->startDefinition();

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 01f0079198c74..16cdb7e577237 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -13012,7 +13012,7 @@ Decl *Sema::ActOnAliasDeclaration(Scope *S, AccessSpecifier AS,
       NewDecl->setInvalidDecl();
     else if (OldDecl) {
       NewDecl->setPreviousDecl(OldDecl);
-      CheckRedeclarationModuleOwnership(NewDecl, OldDecl);
+      CheckRedeclarationInModule(NewDecl, OldDecl);
     }
 
     NewND = NewDecl;

diff  --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 3278ca143dcce..64a0b45feb980 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -2063,7 +2063,7 @@ DeclResult Sema::CheckClassTemplate(
   }
 
   if (PrevClassTemplate)
-    CheckRedeclarationModuleOwnership(NewTemplate, PrevClassTemplate);
+    CheckRedeclarationInModule(NewTemplate, PrevClassTemplate);
 
   if (Invalid) {
     NewTemplate->setInvalidDecl();

diff  --git a/clang/test/CXX/module/module.interface/p2-2.cpp b/clang/test/CXX/module/module.interface/p2-2.cpp
new file mode 100644
index 0000000000000..359e068d230af
--- /dev/null
+++ b/clang/test/CXX/module/module.interface/p2-2.cpp
@@ -0,0 +1,37 @@
+// The intention of this file to check we could only export declarations in namesapce scope.
+//
+// RUN: %clang_cc1 -std=c++20 %s -verify
+
+export module X;
+
+export template <typename T>
+struct X {
+  struct iterator {
+    T node;
+  };
+  void foo() {}
+  template <typename U>
+  U bar();
+};
+
+export template <typename T> X<T>::iterator;                      // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
+export template <typename T> void X<T>::foo();                    // expected-error {{cannot export 'foo' as it is not at namespace scope}}
+export template <typename T> template <typename U> U X<T>::bar(); // expected-error {{cannot export 'bar' as it is not at namespace scope}}
+
+export struct Y {
+  struct iterator {
+    int node;
+  };
+  void foo() {}
+  template <typename U>
+  U bar();
+};
+
+export struct Y::iterator;               // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
+export void Y::foo();                    // expected-error {{cannot export 'foo' as it is not at namespace scope}}
+export template <typename U> U Y::bar(); // expected-error {{cannot export 'bar' as it is not at namespace scope}}
+
+export {
+  template <typename T> X<T>::iterator; // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
+  struct Y::iterator;                   // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
+}

diff  --git a/clang/test/CXX/module/module.interface/p6.cpp b/clang/test/CXX/module/module.interface/p6.cpp
new file mode 100644
index 0000000000000..a696851ccbff4
--- /dev/null
+++ b/clang/test/CXX/module/module.interface/p6.cpp
@@ -0,0 +1,93 @@
+// The test is check we couldn't export a redeclaration which isn't exported previously and
+// check it is OK to redeclare no matter exported nor not if is the previous declaration is exported.
+// RUN: %clang_cc1 -std=c++20 %s -verify
+
+export module X;
+
+struct S { // expected-note {{previous declaration is here}}
+  int n;
+};
+typedef S S;
+export typedef S S; // OK, does not redeclare an entity
+export struct S;    // expected-error {{cannot export redeclaration 'S' here since the previous declaration is not exported}}
+
+namespace A {
+struct X; // expected-note {{previous declaration is here}}
+export struct Y;
+} // namespace A
+
+namespace A {
+export struct X; // expected-error {{cannot export redeclaration 'X' here since the previous declaration is not exported}}
+export struct Y; // OK
+struct Z;        // expected-note {{previous declaration is here}}
+export struct Z; // expected-error {{cannot export redeclaration 'Z' here since the previous declaration is not exported}}
+} // namespace A
+
+namespace A {
+struct B;    // expected-note {{previous declaration is here}}
+struct C {}; // expected-note {{previous declaration is here}}
+} // namespace A
+
+namespace A {
+export struct B {}; // expected-error {{cannot export redeclaration 'B' here since the previous declaration is not exported}}
+export struct C;    // expected-error {{cannot export redeclaration 'C' here since the previous declaration is not exported}}
+} // namespace A
+
+template <typename T>
+struct TemplS; // expected-note {{previous declaration is here}}
+
+export template <typename T>
+struct TemplS {}; // expected-error {{cannot export redeclaration 'TemplS' here since the previous declaration is not exported}}
+
+template <typename T>
+struct TemplS2; // expected-note {{previous declaration is here}}
+
+export template <typename U>
+struct TemplS2 {}; // expected-error {{cannot export redeclaration 'TemplS2' here since the previous declaration is not exported}}
+
+void baz();        // expected-note {{previous declaration is here}}
+export void baz(); // expected-error {{cannot export redeclaration 'baz' here since the previous declaration is not exported}}
+
+namespace A {
+export void foo();
+void bar();        // expected-note {{previous declaration is here}}
+export void bar(); // expected-error {{cannot export redeclaration 'bar' here since the previous declaration is not exported}}
+void f1();         // expected-note {{previous declaration is here}}
+} // namespace A
+
+// OK
+//
+// [module.interface]/p6
+// A redeclaration of an entity X is implicitly exported if X was introduced by an exported declaration
+void A::foo();
+
+// The compiler couldn't export A::f1() here since A::f1() is declared above without exported.
+// See [module.interface]/p6 for details.
+export void A::f1(); // expected-error {{cannot export redeclaration 'f1' here since the previous declaration is not exported}}
+
+template <typename T>
+void TemplFunc(); // expected-note {{previous declaration is here}}
+
+export template <typename T>
+void TemplFunc() { // expected-error {{cannot export redeclaration 'TemplFunc' here since the previous declaration is not exported}}
+}
+
+namespace A {
+template <typename T>
+void TemplFunc2(); // expected-note {{previous declaration is here}}
+export template <typename T>
+void TemplFunc2() {} // expected-error {{cannot export redeclaration 'TemplFunc2' here since the previous declaration is not exported}}
+template <typename T>
+void TemplFunc3(); // expected-note {{previous declaration is here}}
+} // namespace A
+
+export template <typename T>
+void A::TemplFunc3() {} // expected-error {{cannot export redeclaration 'TemplFunc3' here since the previous declaration is not exported}}
+
+int var;        // expected-note {{previous declaration is here}}
+export int var; // expected-error {{cannot export redeclaration 'var' here since the previous declaration is not exported}}
+
+template <typename T>
+T TemplVar; // expected-note {{previous declaration is here}}
+export template <typename T>
+T TemplVar; // expected-error {{cannot export redeclaration 'TemplVar' here since the previous declaration is not exported}}


        


More information about the cfe-commits mailing list