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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
