[llvm-branch-commits] [clang] release/20.x: [modules] Handle friend function that was a definition but became only a declaration during AST deserialization (#132214) (PR #134232)
Dmitry Polukhin via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Apr 3 04:14:20 PDT 2025
https://github.com/dmpolukhin created https://github.com/llvm/llvm-project/pull/134232
Fix for regression https://github.com/llvm/llvm-project/issues/130917, changes in https://github.com/llvm/llvm-project/pull/111992 were too broad. This change reduces scope of previous fix. Added `ExternalASTSource::wasThisDeclarationADefinition` to detect cases when FunctionDecl lost body due to declaration merges.
>From 73ed00f5ef37fc19495bee13d0366fe093c5ac10 Mon Sep 17 00:00:00 2001
From: Dmitry Polukhin <34227995+dmpolukhin at users.noreply.github.com>
Date: Thu, 3 Apr 2025 08:27:13 +0100
Subject: [PATCH 1/2] [modules] Handle friend function that was a definition
but became only a declaration during AST deserialization (#132214)
Fix for regression #130917, changes in #111992 were too broad. This change reduces scope of previous fix. Added `ExternalASTSource::wasThisDeclarationADefinition` to detect cases when FunctionDecl lost body due to declaration merges.
---
clang/include/clang/AST/ExternalASTSource.h | 4 ++
.../clang/Sema/MultiplexExternalSemaSource.h | 2 +
clang/include/clang/Serialization/ASTReader.h | 6 +++
clang/lib/AST/ExternalASTSource.cpp | 4 ++
.../lib/Sema/MultiplexExternalSemaSource.cpp | 8 ++++
.../lib/Sema/SemaTemplateInstantiateDecl.cpp | 12 +++---
clang/lib/Serialization/ASTReader.cpp | 4 ++
clang/lib/Serialization/ASTReaderDecl.cpp | 3 ++
.../friend-default-parameters-modules.cpp | 39 +++++++++++++++++++
.../SemaCXX/friend-default-parameters.cpp | 21 ++++++++++
10 files changed, 98 insertions(+), 5 deletions(-)
create mode 100644 clang/test/SemaCXX/friend-default-parameters-modules.cpp
create mode 100644 clang/test/SemaCXX/friend-default-parameters.cpp
diff --git a/clang/include/clang/AST/ExternalASTSource.h b/clang/include/clang/AST/ExternalASTSource.h
index 42aed56d42e07..f45e3af7602c1 100644
--- a/clang/include/clang/AST/ExternalASTSource.h
+++ b/clang/include/clang/AST/ExternalASTSource.h
@@ -191,6 +191,10 @@ class ExternalASTSource : public RefCountedBase<ExternalASTSource> {
virtual ExtKind hasExternalDefinitions(const Decl *D);
+ /// True if this function declaration was a definition before in its own
+ /// module.
+ virtual bool wasThisDeclarationADefinition(const FunctionDecl *FD);
+
/// Finds all declarations lexically contained within the given
/// DeclContext, after applying an optional filter predicate.
///
diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
index 921bebe3a44af..391c2177d75ec 100644
--- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h
+++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -92,6 +92,8 @@ class MultiplexExternalSemaSource : public ExternalSemaSource {
ExtKind hasExternalDefinitions(const Decl *D) override;
+ bool wasThisDeclarationADefinition(const FunctionDecl *FD) override;
+
/// Find all declarations with the given name in the
/// given context.
bool FindExternalVisibleDeclsByName(const DeclContext *DC,
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index 47301419c76c6..23c98282f228f 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -1392,6 +1392,10 @@ class ASTReader
llvm::DenseMap<const Decl *, bool> DefinitionSource;
+ /// Friend functions that were defined but might have had their bodies
+ /// removed.
+ llvm::DenseSet<const FunctionDecl *> ThisDeclarationWasADefinitionSet;
+
bool shouldDisableValidationForFile(const serialization::ModuleFile &M) const;
/// Reads a statement from the specified cursor.
@@ -2375,6 +2379,8 @@ class ASTReader
ExtKind hasExternalDefinitions(const Decl *D) override;
+ bool wasThisDeclarationADefinition(const FunctionDecl *FD) override;
+
/// Retrieve a selector from the given module with its local ID
/// number.
Selector getLocalSelector(ModuleFile &M, unsigned LocalID);
diff --git a/clang/lib/AST/ExternalASTSource.cpp b/clang/lib/AST/ExternalASTSource.cpp
index e2451f294741d..3e865cb7679b5 100644
--- a/clang/lib/AST/ExternalASTSource.cpp
+++ b/clang/lib/AST/ExternalASTSource.cpp
@@ -38,6 +38,10 @@ ExternalASTSource::hasExternalDefinitions(const Decl *D) {
return EK_ReplyHazy;
}
+bool ExternalASTSource::wasThisDeclarationADefinition(const FunctionDecl *FD) {
+ return false;
+}
+
void ExternalASTSource::FindFileRegionDecls(FileID File, unsigned Offset,
unsigned Length,
SmallVectorImpl<Decl *> &Decls) {}
diff --git a/clang/lib/Sema/MultiplexExternalSemaSource.cpp b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
index 6d945300c386c..fbfb242598c24 100644
--- a/clang/lib/Sema/MultiplexExternalSemaSource.cpp
+++ b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
@@ -107,6 +107,14 @@ MultiplexExternalSemaSource::hasExternalDefinitions(const Decl *D) {
return EK_ReplyHazy;
}
+bool MultiplexExternalSemaSource::wasThisDeclarationADefinition(
+ const FunctionDecl *FD) {
+ for (const auto &S : Sources)
+ if (S->wasThisDeclarationADefinition(FD))
+ return true;
+ return false;
+}
+
bool MultiplexExternalSemaSource::FindExternalVisibleDeclsByName(
const DeclContext *DC, DeclarationName Name,
const DeclContext *OriginalDC) {
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 0c25b87439a95..7158e204db427 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2175,11 +2175,13 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl(
// Friend function defined withing class template may stop being function
// definition during AST merges from different modules, in this case decl
// with function body should be used for instantiation.
- if (isFriend) {
- const FunctionDecl *Defn = nullptr;
- if (D->hasBody(Defn)) {
- D = const_cast<FunctionDecl *>(Defn);
- FunctionTemplate = Defn->getDescribedFunctionTemplate();
+ if (ExternalASTSource *Source = SemaRef.Context.getExternalSource()) {
+ if (isFriend && Source->wasThisDeclarationADefinition(D)) {
+ const FunctionDecl *Defn = nullptr;
+ if (D->hasBody(Defn)) {
+ D = const_cast<FunctionDecl *>(Defn);
+ FunctionTemplate = Defn->getDescribedFunctionTemplate();
+ }
}
}
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 427b3c82c4737..fcf25f5b66e07 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -9664,6 +9664,10 @@ ExternalASTSource::ExtKind ASTReader::hasExternalDefinitions(const Decl *FD) {
return I->second ? EK_Never : EK_Always;
}
+bool ASTReader::wasThisDeclarationADefinition(const FunctionDecl *FD) {
+ return ThisDeclarationWasADefinitionSet.contains(FD);
+}
+
Selector ASTReader::getLocalSelector(ModuleFile &M, unsigned LocalID) {
return DecodeSelector(getGlobalSelectorID(M, LocalID));
}
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index b4ff71c8958a4..2562756db1570 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -520,6 +520,9 @@ void ASTDeclReader::ReadFunctionDefinition(FunctionDecl *FD) {
}
// Store the offset of the body so we can lazily load it later.
Reader.PendingBodies[FD] = GetCurrentCursorOffset();
+ // For now remember ThisDeclarationWasADefinition only for friend functions.
+ if (FD->getFriendObjectKind())
+ Reader.ThisDeclarationWasADefinitionSet.insert(FD);
}
void ASTDeclReader::Visit(Decl *D) {
diff --git a/clang/test/SemaCXX/friend-default-parameters-modules.cpp b/clang/test/SemaCXX/friend-default-parameters-modules.cpp
new file mode 100644
index 0000000000000..9c4aff9f1964a
--- /dev/null
+++ b/clang/test/SemaCXX/friend-default-parameters-modules.cpp
@@ -0,0 +1,39 @@
+// RUN: rm -fR %t
+// RUN: split-file %s %t
+// RUN: cd %t
+// RUN: %clang_cc1 -std=c++20 -fmodule-map-file=modules.map -xc++ -emit-module -fmodule-name=foo modules.map -o foo.pcm
+// RUN: %clang_cc1 -std=c++20 -fmodule-map-file=modules.map -O1 -emit-obj main.cc -verify -fmodule-file=foo.pcm
+
+//--- modules.map
+module "foo" {
+ export *
+ module "foo.h" {
+ export *
+ header "foo.h"
+ }
+}
+
+//--- foo.h
+#pragma once
+
+template <int>
+void Create(const void* = nullptr);
+
+template <int>
+struct ObjImpl {
+ template <int>
+ friend void ::Create(const void*);
+};
+
+template <int I>
+void Create(const void*) {
+ (void) ObjImpl<I>{};
+}
+
+//--- main.cc
+// expected-no-diagnostics
+#include "foo.h"
+
+int main() {
+ Create<42>();
+}
diff --git a/clang/test/SemaCXX/friend-default-parameters.cpp b/clang/test/SemaCXX/friend-default-parameters.cpp
new file mode 100644
index 0000000000000..7190477ac496a
--- /dev/null
+++ b/clang/test/SemaCXX/friend-default-parameters.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -std=c++20 -verify -emit-llvm-only %s
+
+template <int>
+void Create(const void* = nullptr);
+
+template <int>
+struct ObjImpl {
+ template <int>
+ friend void ::Create(const void*);
+};
+
+template <int I>
+void Create(const void*) {
+ (void) ObjImpl<I>{};
+}
+
+int main() {
+ Create<42>();
+}
+
+// expected-no-diagnostics
>From 6e175c3d1e07008d1fe41a49c2ac8a605d368242 Mon Sep 17 00:00:00 2001
From: Dmitry Polukhin <dmitry.polukhin at gmail.com>
Date: Thu, 3 Apr 2025 03:53:23 -0700
Subject: [PATCH 2/2] Add release notes
---
clang/docs/ReleaseNotes.rst | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f4befc242f28b..e57fa9786e6f2 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1065,6 +1065,7 @@ Bug Fixes to C++ Support
- Fixed an incorrect pointer access when checking access-control on concepts. (#GH131530)
- Fixed various alias CTAD bugs involving variadic template arguments. (#GH123591), (#GH127539), (#GH129077),
(#GH129620), and (#GH129998).
+- Fixed the false compilation error "redefinition of default argument" for friend functions with default parameters. (#GH130917)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
More information about the llvm-branch-commits
mailing list