[clang] 76864e6 - [C++20] [Modules] Don't find module for linkage for decls in global

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2024-06-28T16:12:50+08:00
New Revision: 76864e6af134aa240069d42ba15e0b89fd7d6b4c

URL: https://github.com/llvm/llvm-project/commit/76864e6af134aa240069d42ba15e0b89fd7d6b4c
DIFF: https://github.com/llvm/llvm-project/commit/76864e6af134aa240069d42ba15e0b89fd7d6b4c.diff

LOG: [C++20] [Modules] Don't find module for linkage for decls in global
module

Possibly fix https://github.com/llvm/llvm-project/issues/96693

The direct reason is that we are calculating the linkage for the
declaration too early so that the linkage got calculated incorrectly.

And after I look into the problem, I found it is completely not
necessary to calculate the linkage there. It is for ModulesTS. So I
simply removes that legacy experimental code and fix the issue.

Added: 
    clang/test/Modules/forward-friend.cppm

Modified: 
    clang/include/clang/AST/DeclBase.h
    clang/lib/AST/Decl.cpp
    clang/lib/Sema/SemaLookup.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h
index 3310f57acc683..45dac82e54077 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -835,10 +835,7 @@ class alignas(8) Decl {
 
   /// Get the module that owns this declaration for linkage purposes.
   /// There only ever is such a standard C++ module.
-  ///
-  /// \param IgnoreLinkage Ignore the linkage of the entity; assume that
-  /// all declarations in a global module fragment are unowned.
-  Module *getOwningModuleForLinkage(bool IgnoreLinkage = false) const;
+  Module *getOwningModuleForLinkage() const;
 
   /// Determine whether this declaration is definitely visible to name lookup,
   /// independent of whether the owning module is visible.

diff  --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 9d0a835a12c45..16ed6d88d1cb1 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -1614,7 +1614,7 @@ LinkageInfo LinkageComputer::getDeclLinkageAndVisibility(const NamedDecl *D) {
                              : CK);
 }
 
-Module *Decl::getOwningModuleForLinkage(bool IgnoreLinkage) const {
+Module *Decl::getOwningModuleForLinkage() const {
   if (isa<NamespaceDecl>(this))
     // Namespaces never have module linkage.  It is the entities within them
     // that [may] do.
@@ -1637,24 +1637,9 @@ Module *Decl::getOwningModuleForLinkage(bool IgnoreLinkage) const {
 
   case Module::ModuleHeaderUnit:
   case Module::ExplicitGlobalModuleFragment:
-  case Module::ImplicitGlobalModuleFragment: {
-    // External linkage declarations in the global module have no owning module
-    // for linkage purposes. But internal linkage declarations in the global
-    // module fragment of a particular module are owned by that module for
-    // linkage purposes.
-    // FIXME: p1815 removes the need for this distinction -- there are no
-    // internal linkage declarations that need to be referred to from outside
-    // this TU.
-    if (IgnoreLinkage)
-      return nullptr;
-    bool InternalLinkage;
-    if (auto *ND = dyn_cast<NamedDecl>(this))
-      InternalLinkage = !ND->hasExternalFormalLinkage();
-    else
-      InternalLinkage = isInAnonymousNamespace();
-    return InternalLinkage ? M->Kind == Module::ModuleHeaderUnit ? M : M->Parent
-                           : nullptr;
-  }
+  case Module::ImplicitGlobalModuleFragment:
+    // The global module shouldn't change the linkage.
+    return nullptr;
 
   case Module::PrivateModuleFragment:
     // The private module fragment is part of its containing module for linkage

diff  --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 12b13eb8683ac..9a3fabc5933ee 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -5965,7 +5965,7 @@ RedeclarationKind Sema::forRedeclarationInCurContext() const {
   // anything that is not visible. We don't need to check linkage here; if
   // the context has internal linkage, redeclaration lookup won't find things
   // from other TUs, and we can't safely compute linkage yet in general.
-  if (cast<Decl>(CurContext)->getOwningModuleForLinkage(/*IgnoreLinkage*/ true))
+  if (cast<Decl>(CurContext)->getOwningModuleForLinkage())
     return RedeclarationKind::ForVisibleRedeclaration;
   return RedeclarationKind::ForExternalRedeclaration;
 }

diff  --git a/clang/test/Modules/forward-friend.cppm b/clang/test/Modules/forward-friend.cppm
new file mode 100644
index 0000000000000..dfadf4fcc1dae
--- /dev/null
+++ b/clang/test/Modules/forward-friend.cppm
@@ -0,0 +1,22 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/m.cppm -fsyntax-only -verify
+
+//--- foo.h
+
+template <typename... U>
+static void foo(U...) noexcept;
+
+class A {
+  template <typename... U>
+  friend void foo(U...) noexcept;
+};
+
+//--- m.cppm
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module m;
+export using ::A;