-
Notifications
You must be signed in to change notification settings - Fork 14.8k
[clang] remove workaround for SubstNonTypeTemplateParmExpr transform #158541
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[clang] remove workaround for SubstNonTypeTemplateParmExpr transform #158541
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though it doesn't fix our problem (see comment), it's still nice to remove such a workaround, thanks!
297953b
to
61ae933
Compare
@llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) ChangesThis removes a workaround which gets in the way of #141776, including the test changes as expected under that implementation. Full diff: https://github.com/llvm/llvm-project/pull/158541.diff 3 Files Affected:
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index a72c95d6d77cf..9db3b07f71f7b 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2446,40 +2446,14 @@ TemplateInstantiator::TransformSubstNonTypeTemplateParmExpr(
SubstReplacement = TransformExpr(E->getReplacement());
if (SubstReplacement.isInvalid())
return true;
- QualType SubstType = TransformType(E->getParameterType(getSema().Context));
- if (SubstType.isNull())
- return true;
- // The type may have been previously dependent and not now, which means we
- // might have to implicit cast the argument to the new type, for example:
- // template<auto T, decltype(T) U>
- // concept C = sizeof(U) == 4;
- // void foo() requires C<2, 'a'> { }
- // When normalizing foo(), we first form the normalized constraints of C:
- // AtomicExpr(sizeof(U) == 4,
- // U=SubstNonTypeTemplateParmExpr(Param=U,
- // Expr=DeclRef(U),
- // Type=decltype(T)))
- // Then we substitute T = 2, U = 'a' into the parameter mapping, and need to
- // produce:
- // AtomicExpr(sizeof(U) == 4,
- // U=SubstNonTypeTemplateParmExpr(Param=U,
- // Expr=ImpCast(
- // decltype(2),
- // SubstNTTPE(Param=U, Expr='a',
- // Type=char)),
- // Type=decltype(2)))
- // The call to CheckTemplateArgument here produces the ImpCast.
- TemplateArgument SugaredConverted, CanonicalConverted;
- if (SemaRef
- .CheckTemplateArgument(E->getParameter(), SubstType,
- SubstReplacement.get(), SugaredConverted,
- CanonicalConverted,
- /*StrictCheck=*/false, Sema::CTAK_Specified)
- .isInvalid())
+ auto *Param = cast_or_null<NonTypeTemplateParmDecl>(
+ TransformDecl(E->getNameLoc(), E->getParameter()));
+ if (!Param)
return true;
return transformNonTypeTemplateParmRef(
- E->getAssociatedDecl(), E->getParameter(), E->getExprLoc(),
- SugaredConverted, E->getPackIndex(), E->getFinal());
+ E->getAssociatedDecl(), Param, E->getExprLoc(),
+ TemplateArgument(SubstReplacement.get(), /*IsCanonical=*/false),
+ E->getPackIndex(), E->getFinal());
}
ExprResult TemplateInstantiator::RebuildVarDeclRefExpr(ValueDecl *PD,
diff --git a/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp b/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
index 1f4d44218ad1f..2f1817d0ca7eb 100644
--- a/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
+++ b/clang/test/SemaCXX/cxx20-ctad-type-alias.cpp
@@ -113,7 +113,7 @@ using Bar = Foo<X, sizeof(X)>; // expected-note {{candidate template ignored: co
// expected-note {{implicit deduction guide declared as 'template <typename X> requires __is_deducible(test9::Bar, test9::Foo<X, sizeof(X)>) Bar(test9::Foo<X, sizeof(X)>) -> test9::Foo<X, sizeof(X)>'}} \
// expected-note {{implicit deduction guide declared as 'template <typename X> requires __is_deducible(test9::Bar, test9::Foo<X, sizeof(X)>) Bar(const X (&)[sizeof(X)]) -> test9::Foo<X, sizeof(X)>'}} \
// expected-note {{candidate template ignored: constraints not satisfied [with X = int]}} \
- // expected-note {{cannot deduce template arguments for 'test9::Bar' from 'test9::Foo<int, 4UL>'}}
+ // expected-note {{cannot deduce template arguments for 'test9::Bar' from 'test9::Foo<int, sizeof(int)>'}}
Bar s = {{1}}; // expected-error {{no viable constructor or deduction guide }}
diff --git a/clang/test/SemaTemplate/instantiate-template-argument.cpp b/clang/test/SemaTemplate/instantiate-template-argument.cpp
index 43d5d00c8cb20..74050acdf4f35 100644
--- a/clang/test/SemaTemplate/instantiate-template-argument.cpp
+++ b/clang/test/SemaTemplate/instantiate-template-argument.cpp
@@ -9,20 +9,24 @@ concept C2 = C1<Y{}, V>;
// sizeof(U) >= 4 [U = V (decltype(Y{}))]
template<char W>
-constexpr int foo() requires C2<int, W> { return 1; }
+constexpr int foo() requires C2<int, W> { return 1; } // #cand1
// sizeof(U) >= 4 [U = W (decltype(int{}))]
template<char X>
// expected-note@+1{{candidate function}}
-constexpr int foo() requires C1<1, X> && true { return 2; }
+constexpr int foo() requires C1<1, X> && true { return 2; } // #cand2
// sizeof(U) >= 4 [U = X (decltype(1))]
static_assert(foo<'a'>() == 2);
+// expected-error@-1 {{call to 'foo' is ambiguous}}
+// expected-note@#cand1 {{candidate function}}
+// expected-note@#cand2 {{candidate function}}
template<char Z>
-// expected-note@+1{{candidate function}}
-constexpr int foo() requires C2<long long, Z> && true { return 3; }
+constexpr int foo() requires C2<long long, Z> && true { return 3; } // #cand3
// sizeof(U) >= 4 [U = Z (decltype(long long{}))]
static_assert(foo<'a'>() == 3);
-// expected-error@-1{{call to 'foo' is ambiguous}}
\ No newline at end of file
+// expected-error@-1{{call to 'foo' is ambiguous}}
+// expected-note@#cand1 {{candidate function}}
+// expected-note@#cand3 {{candidate function}}
|
This removes a workaround which gets in the way of #141776, including the test changes as expected under that implementation.
61ae933
to
65d24f0
Compare
FYI the test change here is not really correct (and so it is not in #141776, where I have taken it from). I have figured out since that calling the semantic action is necessary here, this is just not modeled correctly in clang. |
This removes a workaround which gets in the way of #141776, including the test changes as expected under that implementation.