Author: Dmitry Polukhin Date: 2025-04-03T08:27:13+01:00 New Revision: e1aaee7ea218f1d89646fa1f43bb4c94c27808b5
URL: https://github.com/llvm/llvm-project/commit/e1aaee7ea218f1d89646fa1f43bb4c94c27808b5 DIFF: https://github.com/llvm/llvm-project/commit/e1aaee7ea218f1d89646fa1f43bb4c94c27808b5.diff LOG: [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. Added: clang/test/SemaCXX/friend-default-parameters-modules.cpp clang/test/SemaCXX/friend-default-parameters.cpp Modified: clang/include/clang/AST/ExternalASTSource.h clang/include/clang/Sema/MultiplexExternalSemaSource.h clang/include/clang/Serialization/ASTReader.h clang/lib/AST/ExternalASTSource.cpp clang/lib/Sema/MultiplexExternalSemaSource.cpp clang/lib/Sema/SemaTemplateInstantiateDecl.cpp clang/lib/Serialization/ASTReader.cpp clang/lib/Serialization/ASTReaderDecl.cpp Removed: ################################################################################ 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 2779b3d1cf2ea..58fcc06c3696d 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. @@ -2374,6 +2378,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 8aaaea0bcdd66..9ea5ecab2d030 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -2604,11 +2604,13 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl( // Friend function defined withing class template may stop being function // definition during AST merges from diff erent 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 58a57d6c54523..8e573a11efd35 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -9661,6 +9661,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 77daeaee5dd1f..b838f84c973de 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -523,6 +523,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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
