[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)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Apr 3 04:14:56 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-modules
Author: Dmitry Polukhin (dmpolukhin)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/134232.diff
11 Files Affected:
- (modified) clang/docs/ReleaseNotes.rst (+1)
- (modified) clang/include/clang/AST/ExternalASTSource.h (+4)
- (modified) clang/include/clang/Sema/MultiplexExternalSemaSource.h (+2)
- (modified) clang/include/clang/Serialization/ASTReader.h (+6)
- (modified) clang/lib/AST/ExternalASTSource.cpp (+4)
- (modified) clang/lib/Sema/MultiplexExternalSemaSource.cpp (+8)
- (modified) clang/lib/Sema/SemaTemplateInstantiateDecl.cpp (+7-5)
- (modified) clang/lib/Serialization/ASTReader.cpp (+4)
- (modified) clang/lib/Serialization/ASTReaderDecl.cpp (+3)
- (added) clang/test/SemaCXX/friend-default-parameters-modules.cpp (+39)
- (added) clang/test/SemaCXX/friend-default-parameters.cpp (+21)
``````````diff
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
^^^^^^^^^^^^^^^^^^^^^^^^^
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
``````````
</details>
https://github.com/llvm/llvm-project/pull/134232
More information about the llvm-branch-commits
mailing list