[clang] 9791b58 - [C++20 Modules] Don't create global module fragment for extern linkage declaration in GMF already

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2021-12-09T13:55:15+08:00
New Revision: 9791b589516b644a6273607b46a9c6661993d667

URL: https://github.com/llvm/llvm-project/commit/9791b589516b644a6273607b46a9c6661993d667
DIFF: https://github.com/llvm/llvm-project/commit/9791b589516b644a6273607b46a9c6661993d667.diff

LOG: [C++20 Modules] Don't create global module fragment for extern linkage declaration in GMF already

Previously we would create global module fragment for extern linkage
declaration which is alreday in global module fragment. However, it is
clearly redundant to do so. This patch would check if the extern linkage
declaration are already in GMF before we create a GMF for it.

Added: 
    clang/test/CXX/module/module.unit/p7/Inputs/h7.h
    clang/test/CXX/module/module.unit/p7/t7.cpp

Modified: 
    clang/include/clang/Sema/Sema.h
    clang/lib/Sema/SemaDeclCXX.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index a12dd4db66c13..73c5ad1dd7acf 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2222,6 +2222,12 @@ class Sema final {
     return ModuleScopes.empty() ? nullptr : ModuleScopes.back().Module;
   }
 
+  /// Helper function to judge if we are in module purview.
+  /// Return false if we are not in a module.
+  bool isCurrentModulePurview() const {
+    return getCurrentModule() ? getCurrentModule()->isModulePurview() : false;
+  }
+
   /// Enter the scope of the global module.
   Module *PushGlobalModuleFragment(SourceLocation BeginLoc, bool IsImplicit);
   /// Leave the scope of the global module.

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 4a0eda2a700fe..3f6bde1b9ed6a 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -16153,7 +16153,10 @@ Decl *Sema::ActOnStartLinkageSpecification(Scope *S, SourceLocation ExternLoc,
   ///   - ...
   ///   - appears within a linkage-specification,
   ///   it is attached to the global module.
-  if (getLangOpts().CPlusPlusModules && getCurrentModule()) {
+  ///
+  /// If the declaration is already in global module fragment, we don't
+  /// need to attach it again.
+  if (getLangOpts().CPlusPlusModules && isCurrentModulePurview()) {
     Module *GlobalModule =
         PushGlobalModuleFragment(ExternLoc, /*IsImplicit=*/true);
     D->setModuleOwnershipKind(Decl::ModuleOwnershipKind::ModulePrivate);
@@ -16177,7 +16180,11 @@ Decl *Sema::ActOnFinishLinkageSpecification(Scope *S,
     LSDecl->setRBraceLoc(RBraceLoc);
   }
 
-  if (getLangOpts().CPlusPlusModules && getCurrentModule())
+  // If the current module doesn't has Parent, it implies that the
+  // LinkageSpec isn't in the module created by itself. So we don't
+  // need to pop it.
+  if (getLangOpts().CPlusPlusModules && getCurrentModule() &&
+      getCurrentModule()->isGlobalModule() && getCurrentModule()->Parent)
     PopGlobalModuleFragment();
 
   PopDeclContext();

diff  --git a/clang/test/CXX/module/module.unit/p7/Inputs/h7.h b/clang/test/CXX/module/module.unit/p7/Inputs/h7.h
new file mode 100644
index 0000000000000..cd3b6706d561c
--- /dev/null
+++ b/clang/test/CXX/module/module.unit/p7/Inputs/h7.h
@@ -0,0 +1,10 @@
+#ifndef H7_H
+#define H7_H
+
+extern "C++" {
+class A {};
+}
+
+class B : public A {};
+
+#endif

diff  --git a/clang/test/CXX/module/module.unit/p7/t7.cpp b/clang/test/CXX/module/module.unit/p7/t7.cpp
new file mode 100644
index 0000000000000..7ce0cbb964131
--- /dev/null
+++ b/clang/test/CXX/module/module.unit/p7/t7.cpp
@@ -0,0 +1,7 @@
+// RUN: rm -fr %t
+// RUN: mkdir %t
+// RUN: %clang_cc1 -std=c++20 -I%S/Inputs/ %s -verify
+// expected-no-diagnostics
+module;
+#include "h7.h"
+export module X;