[clang] [Clang] Avoid transforming lambdas when rebuilding immediate expressions (PR #108693)
Younan Zhang via cfe-commits
cfe-commits at lists.llvm.org
Sat Sep 14 03:49:25 PDT 2024
https://github.com/zyn0217 created https://github.com/llvm/llvm-project/pull/108693
When rebuilding immediate invocations inside `RemoveNestedImmediateInvocation()`, we employed a `TreeTransform` to exercise the traversal. The transformation has a side effect that, for template specialization types, their default template arguments are substituted separately, and if any lambdas are present, they will be transformed into distinct types than those used to instantiate the templates right before the `consteval` handling.
This resulted in `B::func()` getting redundantly instantiated for the case in question. Since we're also in an immediate evaluation context, the body of `foo()` would also get instantiated, so we end up with a spurious friend redefinition error.
Like what we have done in `ComplexRemove`, this patch also avoids the lambda's transformation in TemplateInstantiator if we know we're rebuilding immediate calls. In addition, this patch also consolidates the default argument substitution logic in `CheckTemplateArgumentList()`.
Fixes #107175
>From 4e05a1972e539d08589b44677031f506b0c91203 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Sat, 14 Sep 2024 18:31:42 +0800
Subject: [PATCH] [Clang] Avoid transforming lambdas when rebuilding immediate
expressions
---
clang/docs/ReleaseNotes.rst | 4 +-
clang/lib/Sema/SemaExpr.cpp | 4 +-
clang/lib/Sema/SemaTemplate.cpp | 55 ++++++-------------
clang/lib/Sema/SemaTemplateInstantiate.cpp | 4 ++
.../lib/Sema/SemaTemplateInstantiateDecl.cpp | 10 +++-
clang/test/SemaCXX/cxx2a-consteval.cpp | 24 ++++++++
6 files changed, 57 insertions(+), 44 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 79b154ef1aef5e..022c10e11c545a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -387,8 +387,8 @@ Bug Fixes to C++ Support
- A follow-up fix was added for (#GH61460), as the previous fix was not entirely correct. (#GH86361)
- Fixed a crash in the typo correction of an invalid CTAD guide. (#GH107887)
- Fixed a crash when clang tries to subtitute parameter pack while retaining the parameter
- pack. #GH63819, #GH107560
-
+ pack. (#GH63819), (#GH107560)
+- Avoided a redundant friend declaration instantiation under the certain ``consteval`` contexts. (#GH107175)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 8f3e15cc9a9bb7..2a22b05afe1c34 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -17521,7 +17521,7 @@ static void RemoveNestedImmediateInvocation(
else
break;
}
- /// ConstantExpr are the first layer of implicit node to be removed so if
+ /// ConstantExprs are the first layer of implicit node to be removed so if
/// Init isn't a ConstantExpr, no ConstantExpr will be skipped.
if (auto *CE = dyn_cast<ConstantExpr>(Init);
CE && CE->isImmediateInvocation())
@@ -17534,7 +17534,7 @@ static void RemoveNestedImmediateInvocation(
}
ExprResult TransformLambdaExpr(LambdaExpr *E) {
// Do not rebuild lambdas to avoid creating a new type.
- // Lambdas have already been processed inside their eval context.
+ // Lambdas have already been processed inside their eval contexts.
return E;
}
bool AlwaysRebuild() { return false; }
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index e5ea02a919f4eb..b052afede2cd67 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -5508,50 +5508,31 @@ bool Sema::CheckTemplateArgumentList(
}
// Check whether we have a default argument.
- TemplateArgumentLoc Arg;
+ bool HasDefaultArg;
// Retrieve the default template argument from the template
// parameter. For each kind of template parameter, we substitute the
// template arguments provided thus far and any "outer" template arguments
// (when the template parameter was part of a nested template) into
// the default argument.
- if (TemplateTypeParmDecl *TTP = dyn_cast<TemplateTypeParmDecl>(*Param)) {
- if (!hasReachableDefaultArgument(TTP))
- return diagnoseMissingArgument(*this, TemplateLoc, Template, TTP,
+ TemplateArgumentLoc Arg = SubstDefaultTemplateArgumentIfAvailable(
+ Template, TemplateLoc, RAngleLoc, *Param, SugaredConverted,
+ CanonicalConverted, HasDefaultArg);
+
+ if (Arg.getArgument().isNull()) {
+ if (!HasDefaultArg) {
+ if (TemplateTypeParmDecl *TTP = dyn_cast<TemplateTypeParmDecl>(*Param))
+ return diagnoseMissingArgument(*this, TemplateLoc, Template, TTP,
+ NewArgs);
+ if (NonTypeTemplateParmDecl *NTTP =
+ dyn_cast<NonTypeTemplateParmDecl>(*Param))
+ return diagnoseMissingArgument(*this, TemplateLoc, Template, NTTP,
+ NewArgs);
+ return diagnoseMissingArgument(*this, TemplateLoc, Template,
+ cast<TemplateTemplateParmDecl>(*Param),
NewArgs);
-
- if (SubstDefaultTemplateArgument(*this, Template, TemplateLoc, RAngleLoc,
- TTP, SugaredConverted,
- CanonicalConverted, Arg))
- return true;
- } else if (NonTypeTemplateParmDecl *NTTP
- = dyn_cast<NonTypeTemplateParmDecl>(*Param)) {
- if (!hasReachableDefaultArgument(NTTP))
- return diagnoseMissingArgument(*this, TemplateLoc, Template, NTTP,
- NewArgs);
-
- if (SubstDefaultTemplateArgument(*this, Template, TemplateLoc, RAngleLoc,
- NTTP, SugaredConverted,
- CanonicalConverted, Arg))
- return true;
- } else {
- TemplateTemplateParmDecl *TempParm
- = cast<TemplateTemplateParmDecl>(*Param);
-
- if (!hasReachableDefaultArgument(TempParm))
- return diagnoseMissingArgument(*this, TemplateLoc, Template, TempParm,
- NewArgs);
-
- NestedNameSpecifierLoc QualifierLoc;
- TemplateName Name = SubstDefaultTemplateArgument(
- *this, Template, TemplateLoc, RAngleLoc, TempParm, SugaredConverted,
- CanonicalConverted, QualifierLoc);
- if (Name.isNull())
- return true;
-
- Arg = TemplateArgumentLoc(
- Context, TemplateArgument(Name), QualifierLoc,
- TempParm->getDefaultArgument().getTemplateNameLoc());
+ }
+ return true;
}
// Introduce an instantiation record that describes where we are using
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index c42cc250bb904a..55f38743e2768e 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1673,6 +1673,10 @@ namespace {
}
ExprResult TransformLambdaExpr(LambdaExpr *E) {
+ // Do not rebuild lambdas to avoid creating a new type.
+ // Lambdas have already been processed inside their eval contexts.
+ if (SemaRef.RebuildingImmediateInvocation)
+ return E;
LocalInstantiationScope Scope(SemaRef, /*CombineWithOuterScope=*/true);
Sema::ConstraintEvalRAII<TemplateInstantiator> RAII(*this);
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index bb311e38409280..c3cb9d5d8c2c3d 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6293,9 +6293,13 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc, NamedDecl *D,
if (!SubstRecord) {
// T can be a dependent TemplateSpecializationType when performing a
- // substitution for building a deduction guide.
- assert(CodeSynthesisContexts.back().Kind ==
- CodeSynthesisContext::BuildingDeductionGuides);
+ // substitution for building a deduction guide or for template
+ // argument deduction in the process of rebuilding immediate
+ // expressions. (Because the default argument that involves a lambda
+ // is untransformed and thus could be dependent at this point.)
+ assert(SemaRef.RebuildingImmediateInvocation ||
+ CodeSynthesisContexts.back().Kind ==
+ CodeSynthesisContext::BuildingDeductionGuides);
// Return a nullptr as a sentinel value, we handle it properly in
// the TemplateInstantiator::TransformInjectedClassNameType
// override, which we transform it to a TemplateSpecializationType.
diff --git a/clang/test/SemaCXX/cxx2a-consteval.cpp b/clang/test/SemaCXX/cxx2a-consteval.cpp
index 81923617f637e8..ae331055c52b2e 100644
--- a/clang/test/SemaCXX/cxx2a-consteval.cpp
+++ b/clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -1248,3 +1248,27 @@ void test() {
}
}
+
+// Test that we don't redundantly instantiate the friend declaration in
+// RemoveNestedImmediateInvocation(). Otherwise, we would end up with spurious
+// redefinition errors.
+namespace GH107175 {
+
+consteval void consteval_func() {}
+
+template <auto> struct define_f {
+ friend void foo() {}
+};
+
+template <auto = [] {}> struct A {};
+
+struct B {
+ template <auto T> consteval void func() { (void)define_f<T>{}; }
+};
+
+int main() {
+ B{}.func<A{}>();
+ consteval_func();
+}
+
+} // namespace GH107175
More information about the cfe-commits
mailing list