On 9/20/23 11:03, Patrick Palka wrote:
On Wed, 20 Sep 2023, Patrick Palka wrote:Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk? -- >8 -- This fixes some missed SFINAE in grok_array_decl when checking a C++23 multidimensional subscript operator expression. Note the existing pedwarn code paths are a backward compability fallback for treating invalid a[x, y, z] as a[(x, y, z)], but this should only be done outside of a SFINAE context I think. PR c++/111493 gcc/cp/ChangeLog: * decl2.cc (grok_array_decl): Guard errors with tf_error. In the pedwarn code paths, return error_mark_node when in a SFINAE context. gcc/testsuite/ChangeLog: * g++.dg/cpp23/subscript15.C: New test. --- gcc/cp/decl2.cc | 36 +++++++++++++++--------- gcc/testsuite/g++.dg/cpp23/subscript15.C | 24 ++++++++++++++++ 2 files changed, 47 insertions(+), 13 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp23/subscript15.C diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc index b402befba6d..6eb6d8c57d6 100644 --- a/gcc/cp/decl2.cc +++ b/gcc/cp/decl2.cc @@ -477,7 +477,9 @@ grok_array_decl (location_t loc, tree array_expr, tree index_exp, { /* If it would be valid albeit deprecated expression in C++20, just pedwarn on it and treat it as if wrapped - in (). */ + in () unless we're in a SFINAE context. */ + if (!(complain & tf_error)) + return error_mark_node;It occurred to me that we could check for tf_error much earlier, before we call build_x_compound_expr_from_vec and build_new_op, since they're only used here to implement the backward compatibilty fallback. Perhaps the following is better, then:
This version is OK.
-- >8 -- Subject: [PATCH] c++: missing SFINAE in grok_array_decl [PR111493] We should guard both the diagnostic and backward compatibilty fallback code with tf_error, so that in a SFINAE context we don't issue any diagnostics and correctly recognize ill-formed C++23 multidimensional subscript operator expressions. PR c++/111493 gcc/cp/ChangeLog: * decl2.cc (grok_array_decl): Guard diagnostic and backward compatibility fallback code paths with tf_error. gcc/testsuite/ChangeLog: * g++.dg/cpp23/subscript15.C: New test. --- gcc/cp/decl2.cc | 15 +++++++++++--- gcc/testsuite/g++.dg/cpp23/subscript15.C | 25 ++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp23/subscript15.C diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc index b402befba6d..6ac27cbc15f 100644 --- a/gcc/cp/decl2.cc +++ b/gcc/cp/decl2.cc @@ -459,7 +459,10 @@ grok_array_decl (location_t loc, tree array_expr, tree index_exp, { expr = build_op_subscript (loc, array_expr, index_exp_list, &overload, complain & tf_decltype); - if (expr == error_mark_node) + if (expr == error_mark_node + /* Don't do the backward compatibility fallback in a SFINAE + context. */ + && (complain & tf_error)) { tree idx = build_x_compound_expr_from_vec (*index_exp_list, NULL, tf_none); @@ -510,6 +513,11 @@ grok_array_decl (location_t loc, tree array_expr, tree index_exp,if (index_exp == NULL_TREE){ + if (!(complain & tf_error)) + /* Don't do the backward compatibility fallback in a SFINAE + context. */ + return error_mark_node; + if ((*index_exp_list)->is_empty ()) { error_at (loc, "built-in subscript operator without expression " @@ -561,8 +569,9 @@ grok_array_decl (location_t loc, tree array_expr, tree index_exp, swapped = true, array_expr = p2, index_exp = i1; else { - error_at (loc, "invalid types %<%T[%T]%> for array subscript", - type, TREE_TYPE (index_exp)); + if (complain & tf_error) + error_at (loc, "invalid types %<%T[%T]%> for array subscript", + type, TREE_TYPE (index_exp)); return error_mark_node; }diff --git a/gcc/testsuite/g++.dg/cpp23/subscript15.C b/gcc/testsuite/g++.dg/cpp23/subscript15.Cnew file mode 100644 index 00000000000..fece96be96b --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp23/subscript15.C @@ -0,0 +1,25 @@ +// PR c++/111493 +// { dg-do compile { target c++23 } } + +template<class T, class... Ts> +concept CartesianIndexable = requires(T t, Ts... ts) { t[ts...]; }; + +static_assert(!CartesianIndexable<int>); +static_assert(!CartesianIndexable<int, int>); +static_assert(!CartesianIndexable<int, int, int>); + +static_assert(!CartesianIndexable<int*>); +static_assert(CartesianIndexable<int*, int>); +static_assert(!CartesianIndexable<int*, int, int>); +static_assert(!CartesianIndexable<int*, int*>); + +template<class... Ts> +struct A { + void operator[](Ts...); +}; + +static_assert(!CartesianIndexable<A<>, int>); +static_assert(CartesianIndexable<A<int>, int>); +static_assert(!CartesianIndexable<A<int>>); +static_assert(!CartesianIndexable<A<int>, int, int>); +static_assert(CartesianIndexable<A<int, int>, int, int>);
