[clang] stop error recovery in SFINAE for narrowing in converted constant expressions#183614
[clang] stop error recovery in SFINAE for narrowing in converted constant expressions#183614
Conversation
|
@llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) ChangesA narrowing conversion in a converted constant expression should produce an invalid expression so that [temp.deduct.general]p7 is satisfied, by stopping substitution at this point. This regression was introduced in #164703, and this will be backported to clang-22, so no release notes. Fixes #167709 Full diff: https://github.com/llvm/llvm-project/pull/183614.diff 2 Files Affected:
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 41b0b64409fd5..9db537696e548 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -6470,6 +6470,12 @@ static ExprResult BuildConvertedConstantExpression(Sema &S, Expr *From,
S.Diag(From->getBeginLoc(), diag::ext_cce_narrowing)
<< CCE << /*Constant*/ 1
<< PreNarrowingValue.getAsString(S.Context, PreNarrowingType) << T;
+ // If this is an SFINAE Context, treat the result as invalid so it stops
+ // substitution at this point, respecting C++26 [temp.deduct.general]p7.
+ // FIXME: Should do this whenever the above diagnostic is an error, but
+ // without further changes this would degrade some other diagnostics.
+ if (S.isSFINAEContext())
+ return ExprError();
break;
case NK_Dependent_Narrowing:
@@ -6485,7 +6491,8 @@ static ExprResult BuildConvertedConstantExpression(Sema &S, Expr *From,
// constant expression.
S.Diag(From->getBeginLoc(), diag::ext_cce_narrowing)
<< CCE << /*Constant*/ 0 << From->getType() << T;
- break;
+ if (S.isSFINAEContext())
+ return ExprError();
}
if (!ReturnPreNarrowingValue)
PreNarrowingValue = {};
diff --git a/clang/test/SemaTemplate/temp_arg_nontype_cxx11.cpp b/clang/test/SemaTemplate/temp_arg_nontype_cxx11.cpp
index 45bdb4c623dfe..0b785700ee238 100644
--- a/clang/test/SemaTemplate/temp_arg_nontype_cxx11.cpp
+++ b/clang/test/SemaTemplate/temp_arg_nontype_cxx11.cpp
@@ -43,7 +43,7 @@ void TempFunc() {}
void Useage() {
//expected-error@+2 {{no matching function}}
- //expected-note@-4 {{candidate template ignored: substitution failure [with a = 1, b = 4294967295, c = 1]: non-type template argument evaluates to -1, which cannot be narrowed to type 'unsigned int'}}
+ //expected-note@-4 {{candidate template ignored: invalid explicitly-specified argument for template parameter 'b'}}
TempFunc<1, -1, 1>();
}
}
@@ -114,3 +114,12 @@ void lookup() {
Kolumn<&container::a>().ls(); // expected-error {{<&container::a}}
Kolumn<nullptr>().ls(); // expected-error {{<nullptr}}
}
+
+namespace GH167709 {
+ template <unsigned I> struct A {
+ static_assert(false, "shouldn't instantiate this");
+ };
+ template <int> void f() {}
+ template <int I> typename A<I>::type f() = delete;
+ template void f<-1>();
+} // namespace GH167709
|
…tant expressions A narrowing conversion in a converted constant expression should produce an invalid expression so that [temp.deduct.general]p7 is satisfied, by stopping substitution at this point. Fixes #167709
593c77c to
25c946c
Compare
zyn0217
left a comment
There was a problem hiding this comment.
One nit, though I still wish we would have a better way to not degrade diagnostics.
| << CCE << /*Constant*/ 1 | ||
| << PreNarrowingValue.getAsString(S.Context, PreNarrowingType) << T; | ||
| // If this is an SFINAE Context, treat the result as invalid so it stops | ||
| // substitution at this point, respecting C++26 [temp.deduct.general]p7. |
There was a problem hiding this comment.
Can we expand what temp.deduct.general.p7 says?
There was a problem hiding this comment.
That is a giant paragraph with multiple bullets, and the only relevant part is "The substitution proceeds in lexical order and stops when a condition that causes deduction to fail is encountered".
But that seems well explained enough by the comment?
There was a problem hiding this comment.
Anyway, I am going to merge, we can adjust the comment later with something sensible if we come up with it.
It's mostly, with an exception, about missing notes because these are not emitted through different paths. Another thing we should consider is to just make these diagnostics an error, and not allow them as an extension through a warning anymore, but I am not sure about the impact. We can't back port that either way. |
This workaround is needed to fix "parameter pack may not be accessed at an out of bounds index" compile errors in bitcoin/bitcoin#29409 due in sanitizer jobs due to bitcoin/bitcoin#34660 which updates sanitizers to use LLVM 22 instead of LLVM 21: https://github.com/bitcoin/bitcoin/actions/runs/22501264518/job/65188721708?pr=29409 https://github.com/bitcoin/bitcoin/actions/runs/22501264518/job/65188721733?pr=29409 ```c++ /cxx_build/include/c++/v1/__fwd/tuple.h:40:52: error: a parameter pack may not be accessed at an out of bounds index 40 | using type _LIBCPP_NODEBUG = __type_pack_element<_Ip, _Tp...>; | ^ /cxx_build/include/c++/v1/__utility/try_key_extraction.h:82:67: note: in instantiation of template class 'std::tuple_element<0, std::tuple<>>' requested here 82 | is_same<__remove_const_ref_t<typename tuple_element<0, _Tuple1>::type>, _KeyT>::value, | ^ ``` The upstream LLVM bug is llvm/llvm-project#167709 and fix is llvm/llvm-project#183614
…tant expressions (llvm#183614) A narrowing conversion in a converted constant expression should produce an invalid expression so that [temp.deduct.general]p7 is satisfied, by stopping substitution at this point. This regression was introduced in llvm#164703, and this will be backported to clang-22, so no release notes. Fixes llvm#167709
f61af48 type-map: Work around LLVM 22 "out of bounds index" error (Ryan Ofsky) Pull request description: This workaround is needed to fix "parameter pack may not be accessed at an out of bounds index" compile errors in bitcoin/bitcoin#29409 due in sanitizer jobs due to bitcoin/bitcoin#34660 which updates sanitizers to use LLVM 22 instead of LLVM 21: https://github.com/bitcoin/bitcoin/actions/runs/22501264518/job/65188721708?pr=29409 https://github.com/bitcoin/bitcoin/actions/runs/22501264518/job/65188721733?pr=29409 ```c++ /cxx_build/include/c++/v1/__fwd/tuple.h:40:52: error: a parameter pack may not be accessed at an out of bounds index 40 | using type _LIBCPP_NODEBUG = __type_pack_element<_Ip, _Tp...>; | ^ /cxx_build/include/c++/v1/__utility/try_key_extraction.h:82:67: note: in instantiation of template class 'std::tuple_element<0, std::tuple<>>' requested here 82 | is_same<__remove_const_ref_t<typename tuple_element<0, _Tuple1>::type>, _KeyT>::value, | ^ ``` The upstream LLVM bug is llvm/llvm-project#167709 and fix is llvm/llvm-project#183614 ACKs for top commit: hebasto: ACK f61af48, I have reviewed the code and it looks OK. Tree-SHA512: 19eec5cd304f6e585626fe39a85a82ef2b9b2f5ab1ca8b81c0836d76d3846eb9d4893c0766248251d9e685d49d4a5e84141377dad13888fa8268d78f5c9f7be5
A narrowing conversion in a converted constant expression should produce an invalid expression so that [temp.deduct.general]p7 is satisfied, by stopping substitution at this point.
This regression was introduced in #164703, and this will be backported to clang-22, so no release notes.
Fixes #167709