[clang] dd222ff - [Clang] Avoid transforming lambdas when rebuilding immediate expressions (#108693)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 18 01:34:58 PDT 2024
Author: Younan Zhang
Date: 2024-09-18T16:34:55+08:00
New Revision: dd222ff25129f4d67473a9af598a78d0adfcfd29
URL: https://github.com/llvm/llvm-project/commit/dd222ff25129f4d67473a9af598a78d0adfcfd29
DIFF: https://github.com/llvm/llvm-project/commit/dd222ff25129f4d67473a9af598a78d0adfcfd29.diff
LOG: [Clang] Avoid transforming lambdas when rebuilding immediate expressions (#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
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/SemaCXX/cxx2a-consteval.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3ed9a2984a38fe..dd004228b679e4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -395,8 +395,9 @@ 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)
- Fix a crash when a static assert declaration has an invalid close location. (#GH108687)
+- Avoided a redundant friend declaration instantiation under a certain ``consteval`` context. (#GH107175)
- Fixed an assertion failure in debug mode, and potential crashes in release mode, when
diagnosing a failed cast caused indirectly by a failed implicit conversion to the type of the constructor parameter.
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 80c252c79e4d7a..2f7e9c754ce095 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -17557,7 +17557,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())
@@ -17570,7 +17570,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 e97a7d768b931b..e055c87e783813 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6294,9 +6294,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