https://github.com/dmpolukhin updated
https://github.com/llvm/llvm-project/pull/132214
>From 91e057bf990e2c454b897982ed0b4e823bb3faba Mon Sep 17 00:00:00 2001
From: Dmitry Polukhin <dmitry.poluk...@gmail.com>
Date: Thu, 20 Mar 2025 06:51:46 -0700
Subject: [PATCH 1/7] [clang] Fix for regression #130917
Changes in #111992 was too broad. This change reduces scope of previous
fix. Unfortunately in clang there is no way to know when redeclaration
was craeted artificially due to AST mergse and when it was the case in
the original code.
---
.../lib/Sema/SemaTemplateInstantiateDecl.cpp | 2 +-
.../SemaCXX/friend-default-parameters.cpp | 21 +++++++++++++++++++
2 files changed, 22 insertions(+), 1 deletion(-)
create mode 100644 clang/test/SemaCXX/friend-default-parameters.cpp
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 539c2fdb83797..eda5d1151ab19 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2572,7 +2572,7 @@ 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) {
+ if (isFriend && D->hasOwningModule()) {
const FunctionDecl *Defn = nullptr;
if (D->hasBody(Defn)) {
D = const_cast<FunctionDecl *>(Defn);
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 adf6dfde1c360747fca2befd40e3d225cfeb4970 Mon Sep 17 00:00:00 2001
From: Dmitry Polukhin <dmitry.poluk...@gmail.com>
Date: Tue, 25 Mar 2025 07:14:56 -0700
Subject: [PATCH 2/7] Add test case with C++ modules
---
.../friend-default-parameters-modules.cpp | 39 +++++++++++++++++++
1 file changed, 39 insertions(+)
create mode 100644 clang/test/SemaCXX/friend-default-parameters-modules.cpp
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>();
+}
>From a564da2384ae1412647d1134aaf5359fe30e75a7 Mon Sep 17 00:00:00 2001
From: Dmitry Polukhin <dmitry.poluk...@gmail.com>
Date: Tue, 25 Mar 2025 08:26:49 -0700
Subject: [PATCH 3/7] Add flag to FunctionDecl
---
clang/include/clang/AST/Decl.h | 12 ++++++++++++
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 2 +-
clang/lib/Serialization/ASTReader.cpp | 1 +
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index efac36e49351e..9fdba31eb34ae 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2005,6 +2005,10 @@ class FunctionDecl : public DeclaratorDecl,
unsigned ODRHash;
+ /// Indicates if given function declaration was a definition but its body
+ /// was removed due to declaration merging.
+ bool ThisDeclarationWasADefinition : 1;
+
/// End part of this FunctionDecl's source range.
///
/// We could compute the full range in getSourceRange(). However, when we're
@@ -2190,6 +2194,14 @@ class FunctionDecl : public DeclaratorDecl,
return hasBody(Definition);
}
+ void setThisDeclarationWasADefinition() {
+ ThisDeclarationWasADefinition = true;
+ }
+
+ bool wasThisDeclarationADefinition() {
+ return ThisDeclarationWasADefinition;
+ }
+
/// Returns whether the function has a trivial body that does not require any
/// specific codegen.
bool hasTrivialBody() const;
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index eda5d1151ab19..90e81ab68f0f9 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2572,7 +2572,7 @@ 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 && D->hasOwningModule()) {
+ if (isFriend && D->wasThisDeclarationADefinition()) {
const FunctionDecl *Defn = nullptr;
if (D->hasBody(Defn)) {
D = const_cast<FunctionDecl *>(Defn);
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 2b03446aaa30e..0f676bbddab3f 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -10432,6 +10432,7 @@ void ASTReader::finishPendingActions() {
if (!getContext().getLangOpts().Modules || !FD->hasBody(Defn)) {
FD->setLazyBody(PB->second);
} else {
+ FD->setThisDeclarationWasADefinition();
auto *NonConstDefn = const_cast<FunctionDecl*>(Defn);
mergeDefinitionVisibility(NonConstDefn, FD);
>From 2b42b4d256f9abbc578aa8e92d21d02d50c83679 Mon Sep 17 00:00:00 2001
From: Dmitry Polukhin <dmitry.poluk...@gmail.com>
Date: Tue, 25 Mar 2025 08:32:47 -0700
Subject: [PATCH 4/7] Fix formatting
---
clang/include/clang/AST/Decl.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 9fdba31eb34ae..0d9760d3608b4 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2198,9 +2198,7 @@ class FunctionDecl : public DeclaratorDecl,
ThisDeclarationWasADefinition = true;
}
- bool wasThisDeclarationADefinition() {
- return ThisDeclarationWasADefinition;
- }
+ bool wasThisDeclarationADefinition() { return ThisDeclarationWasADefinition;
}
/// Returns whether the function has a trivial body that does not require any
/// specific codegen.
>From 8d5ecfd15ef1ac8fe576f515454eb683ad55c3f4 Mon Sep 17 00:00:00 2001
From: Dmitry Polukhin <dmitry.poluk...@gmail.com>
Date: Tue, 25 Mar 2025 10:05:38 -0700
Subject: [PATCH 5/7] Add missing initialization
---
clang/lib/AST/Decl.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index e8aeacf24374f..aad54afa5488f 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -3083,6 +3083,7 @@ FunctionDecl::FunctionDecl(Kind DK, ASTContext &C,
DeclContext *DC,
static_cast<unsigned char>(DeductionCandidate::Normal);
FunctionDeclBits.HasODRHash = false;
FunctionDeclBits.FriendConstraintRefersToEnclosingTemplate = false;
+ ThisDeclarationWasADefinition = false;
if (TrailingRequiresClause)
setTrailingRequiresClause(TrailingRequiresClause);
}
>From df9fd8b4ea54779f0495ee666dfdebcc440e2215 Mon Sep 17 00:00:00 2001
From: Dmitry Polukhin <dmitry.poluk...@gmail.com>
Date: Fri, 28 Mar 2025 02:39:01 -0700
Subject: [PATCH 6/7] Move ThisDeclarationWasADefinition to ExternalASTSource
---
clang/include/clang/AST/Decl.h | 10 --------
clang/include/clang/AST/ExternalASTSource.h | 4 +++
clang/include/clang/Serialization/ASTReader.h | 16 +++++++++++-
clang/lib/AST/Decl.cpp | 1 -
clang/lib/AST/ExternalASTSource.cpp | 4 +++
.../lib/Sema/SemaTemplateInstantiateDecl.cpp | 12 +++++----
clang/lib/Serialization/ASTReader.cpp | 10 ++++++--
clang/lib/Serialization/ASTReaderDecl.cpp | 25 ++++++++++++-------
8 files changed, 54 insertions(+), 28 deletions(-)
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 0d9760d3608b4..efac36e49351e 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2005,10 +2005,6 @@ class FunctionDecl : public DeclaratorDecl,
unsigned ODRHash;
- /// Indicates if given function declaration was a definition but its body
- /// was removed due to declaration merging.
- bool ThisDeclarationWasADefinition : 1;
-
/// End part of this FunctionDecl's source range.
///
/// We could compute the full range in getSourceRange(). However, when we're
@@ -2194,12 +2190,6 @@ class FunctionDecl : public DeclaratorDecl,
return hasBody(Definition);
}
- void setThisDeclarationWasADefinition() {
- ThisDeclarationWasADefinition = true;
- }
-
- bool wasThisDeclarationADefinition() { return ThisDeclarationWasADefinition;
}
-
/// Returns whether the function has a trivial body that does not require any
/// specific codegen.
bool hasTrivialBody() const;
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/Serialization/ASTReader.h
b/clang/include/clang/Serialization/ASTReader.h
index 2779b3d1cf2ea..e0b8361cff4f4 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -1390,7 +1390,19 @@ class ASTReader
/// predefines buffer may contain additional definitions.
std::string SuggestedPredefines;
- llvm::DenseMap<const Decl *, bool> DefinitionSource;
+ struct DefinitionSourceFlags {
+ ExtKind HasExternalDefinitions : 2;
+
+ /// Indicates if given function declaration was a definition but its body
+ /// was removed due to declaration merging.
+ bool ThisDeclarationWasADefinition : 1;
+
+ DefinitionSourceFlags()
+ : HasExternalDefinitions(EK_ReplyHazy),
+ ThisDeclarationWasADefinition(false) {}
+ };
+
+ llvm::DenseMap<const Decl *, DefinitionSourceFlags> DefinitionSource;
bool shouldDisableValidationForFile(const serialization::ModuleFile &M)
const;
@@ -2374,6 +2386,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/Decl.cpp b/clang/lib/AST/Decl.cpp
index aad54afa5488f..e8aeacf24374f 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -3083,7 +3083,6 @@ FunctionDecl::FunctionDecl(Kind DK, ASTContext &C,
DeclContext *DC,
static_cast<unsigned char>(DeductionCandidate::Normal);
FunctionDeclBits.HasODRHash = false;
FunctionDeclBits.FriendConstraintRefersToEnclosingTemplate = false;
- ThisDeclarationWasADefinition = false;
if (TrailingRequiresClause)
setTrailingRequiresClause(TrailingRequiresClause);
}
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/SemaTemplateInstantiateDecl.cpp
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 90e81ab68f0f9..011bc949a7d3c 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2572,11 +2572,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 && D->wasThisDeclarationADefinition()) {
- 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 0f676bbddab3f..d8eefbf76fe38 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -9657,7 +9657,14 @@ ExternalASTSource::ExtKind
ASTReader::hasExternalDefinitions(const Decl *FD) {
auto I = DefinitionSource.find(FD);
if (I == DefinitionSource.end())
return EK_ReplyHazy;
- return I->second ? EK_Never : EK_Always;
+ return I->second.HasExternalDefinitions;
+}
+
+bool ASTReader::wasThisDeclarationADefinition(const FunctionDecl *FD) {
+ auto I = DefinitionSource.find(FD);
+ if (I == DefinitionSource.end())
+ return false;
+ return I->second.ThisDeclarationWasADefinition;
}
Selector ASTReader::getLocalSelector(ModuleFile &M, unsigned LocalID) {
@@ -10432,7 +10439,6 @@ void ASTReader::finishPendingActions() {
if (!getContext().getLangOpts().Modules || !FD->hasBody(Defn)) {
FD->setLazyBody(PB->second);
} else {
- FD->setThisDeclarationWasADefinition();
auto *NonConstDefn = const_cast<FunctionDecl*>(Defn);
mergeDefinitionVisibility(NonConstDefn, FD);
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index c3341e00bacef..64ea5f7a319e5 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -512,9 +512,11 @@ uint64_t ASTDeclReader::GetCurrentCursorOffset() {
void ASTDeclReader::ReadFunctionDefinition(FunctionDecl *FD) {
if (Record.readInt()) {
- Reader.DefinitionSource[FD] =
- Loc.F->Kind == ModuleKind::MK_MainFile ||
- Reader.getContext().getLangOpts().BuildingPCHWithObjectFile;
+ Reader.DefinitionSource[FD].HasExternalDefinitions =
+ (Loc.F->Kind == ModuleKind::MK_MainFile ||
+ Reader.getContext().getLangOpts().BuildingPCHWithObjectFile)
+ ? ExternalASTSource::EK_Never
+ : ExternalASTSource::EK_Always;
}
if (auto *CD = dyn_cast<CXXConstructorDecl>(FD)) {
CD->setNumCtorInitializers(Record.readInt());
@@ -523,6 +525,7 @@ void ASTDeclReader::ReadFunctionDefinition(FunctionDecl
*FD) {
}
// Store the offset of the body so we can lazily load it later.
Reader.PendingBodies[FD] = GetCurrentCursorOffset();
+ Reader.DefinitionSource[FD].ThisDeclarationWasADefinition = true;
}
void ASTDeclReader::Visit(Decl *D) {
@@ -1652,9 +1655,11 @@ RedeclarableResult
ASTDeclReader::VisitVarDeclImpl(VarDecl *VD) {
VD->setLocalExternDecl();
if (DefGeneratedInModule) {
- Reader.DefinitionSource[VD] =
- Loc.F->Kind == ModuleKind::MK_MainFile ||
- Reader.getContext().getLangOpts().BuildingPCHWithObjectFile;
+ Reader.DefinitionSource[VD].HasExternalDefinitions =
+ (Loc.F->Kind == ModuleKind::MK_MainFile ||
+ Reader.getContext().getLangOpts().BuildingPCHWithObjectFile)
+ ? ExternalASTSource::EK_Never
+ : ExternalASTSource::EK_Always;
}
if (VD->hasAttr<BlocksAttr>()) {
@@ -1996,9 +2001,11 @@ void ASTDeclReader::ReadCXXDefinitionData(
Data.HasODRHash = true;
if (Record.readInt()) {
- Reader.DefinitionSource[D] =
- Loc.F->Kind == ModuleKind::MK_MainFile ||
- Reader.getContext().getLangOpts().BuildingPCHWithObjectFile;
+ Reader.DefinitionSource[D].HasExternalDefinitions =
+ (Loc.F->Kind == ModuleKind::MK_MainFile ||
+ Reader.getContext().getLangOpts().BuildingPCHWithObjectFile)
+ ? ExternalASTSource::EK_Never
+ : ExternalASTSource::EK_Always;
}
Record.readUnresolvedSet(Data.Conversions);
>From d7234be3d9c63bdbda4ae6270085c8128afc59d3 Mon Sep 17 00:00:00 2001
From: Dmitry Polukhin <dmitry.poluk...@gmail.com>
Date: Fri, 28 Mar 2025 04:31:53 -0700
Subject: [PATCH 7/7] Rename + add only friend functions to the map
---
clang/include/clang/Serialization/ASTReader.h | 6 +++---
clang/lib/Serialization/ASTReaderDecl.cpp | 4 +++-
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/Serialization/ASTReader.h
b/clang/include/clang/Serialization/ASTReader.h
index e0b8361cff4f4..53712e73b04d3 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -1390,19 +1390,19 @@ class ASTReader
/// predefines buffer may contain additional definitions.
std::string SuggestedPredefines;
- struct DefinitionSourceFlags {
+ struct DefinitionSourceBits {
ExtKind HasExternalDefinitions : 2;
/// Indicates if given function declaration was a definition but its body
/// was removed due to declaration merging.
bool ThisDeclarationWasADefinition : 1;
- DefinitionSourceFlags()
+ DefinitionSourceBits()
: HasExternalDefinitions(EK_ReplyHazy),
ThisDeclarationWasADefinition(false) {}
};
- llvm::DenseMap<const Decl *, DefinitionSourceFlags> DefinitionSource;
+ llvm::DenseMap<const Decl *, DefinitionSourceBits> DefinitionSource;
bool shouldDisableValidationForFile(const serialization::ModuleFile &M)
const;
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 64ea5f7a319e5..3f3429f374545 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -525,7 +525,9 @@ void ASTDeclReader::ReadFunctionDefinition(FunctionDecl
*FD) {
}
// Store the offset of the body so we can lazily load it later.
Reader.PendingBodies[FD] = GetCurrentCursorOffset();
- Reader.DefinitionSource[FD].ThisDeclarationWasADefinition = true;
+ // For now remember ThisDeclarationWasADefinition only for friend functions.
+ if (FD->getFriendObjectKind())
+ Reader.DefinitionSource[FD].ThisDeclarationWasADefinition = true;
}
void ASTDeclReader::Visit(Decl *D) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
