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+dmpoluk...@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.poluk...@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 ^^^^^^^^^^^^^^^^^^^^^^^^^ _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
