[clang] e1aaee7 - [modules] Handle friend function that was a definition but became only a declaration during AST deserialization (#132214)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 3 00:27:15 PDT 2025
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
More information about the cfe-commits
mailing list