[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)
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 {
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 DefinitionSource;
+ /// Friend functions that were defined but might have had their bodies
+ /// removed.
+ llvm::DenseSet 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 &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
[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)
dmpolukhin wrote: @nikic what do you mean by ABI change in this case? It doesn't change ABI of generated code, moreover it doesn't even change PCM serialized format because it is in memory only filed and attribute. https://github.com/llvm/llvm-project/pull/134232 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[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)
https://github.com/dmpolukhin closed https://github.com/llvm/llvm-project/pull/134232 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[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)
dmpolukhin wrote: > > @nikic what do you mean by ABI change in this case? It doesn't change ABI > > of generated code, moreover it doesn't even change PCM serialized format > > because it is in memory only filed and attribute. > > It changes the ABI of libclang-cpp, by changing the layout of an exported > type. It looks a bit strange requirement to me and significantly reduces ability to fix regression in release compilers. But if it is the release requirement, this fix cannot be cherry-pick to clang-20 so I abandon this PR. https://github.com/llvm/llvm-project/pull/134232 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[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)
https://github.com/dmpolukhin updated
https://github.com/llvm/llvm-project/pull/134232
>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 {
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 DefinitionSource;
+ /// Friend functions that were defined but might have had their bodies
+ /// removed.
+ llvm::DenseSet 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 &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(
