Re: [PATCH] c++/c-common: Fix convert_vector_to_array_for_subscript for qualified vector types [PR89224]

[Jason Merrill]

On 4/30/24 12:04, Andrew Pinski wrote:
On Tue, Apr 30, 2024 at 11:54 AM Jason Merrill <ja...@redhat.com> wrote:

On 2/20/24 19:06, Andrew Pinski wrote:
After r7-987-gf17a223de829cb, the access for the elements of a vector type 
would lose the qualifiers.
So if we had `constvector[0]`, the type of the element of the array would not 
have const on it.
This was due to a missing build_qualified_type for the inner type of the vector 
when building the array type.
We need to add back the call to build_qualified_type and now the access has the 
correct qualifiers. So the
overloads and even if it is a lvalue or rvalue is correctly done.

Note we correctly now reject the testcase gcc.dg/pr83415.c which was 
incorrectly accepted after r7-987-gf17a223de829cb.

Built and tested for aarch64-linux-gnu.

       PR c++/89224

gcc/c-family/ChangeLog:

       * c-common.cc (convert_vector_to_array_for_subscript): Call 
build_qualified_type
       for the inner type.

gcc/cp/ChangeLog:

       * constexpr.cc (cxx_eval_array_reference): Compare main variants
       for the vector/array types instead of the types directly.

gcc/testsuite/ChangeLog:

       * g++.dg/torture/vector-subaccess-1.C: New test.
       * gcc.dg/pr83415.c: Change warning to error.

Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
---
   gcc/c-family/c-common.cc                      |  7 +++++-
   gcc/cp/constexpr.cc                           |  3 ++-
   .../g++.dg/torture/vector-subaccess-1.C       | 23 +++++++++++++++++++
   gcc/testsuite/gcc.dg/pr83415.c                |  2 +-
   4 files changed, 32 insertions(+), 3 deletions(-)
   create mode 100644 gcc/testsuite/g++.dg/torture/vector-subaccess-1.C

diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
index e15eff698df..884dd9043f9 100644
--- a/gcc/c-family/c-common.cc
+++ b/gcc/c-family/c-common.cc
@@ -8936,6 +8936,7 @@ convert_vector_to_array_for_subscript (location_t loc,
     if (gnu_vector_type_p (TREE_TYPE (*vecp)))
       {
         tree type = TREE_TYPE (*vecp);
+      tree newitype;

         ret = !lvalue_p (*vecp);

@@ -8950,8 +8951,12 @@ convert_vector_to_array_for_subscript (location_t loc,
        for function parameters.  */
         c_common_mark_addressable_vec (*vecp);

+      /* Make sure qualifiers are copied from the vector type to the new 
element
+      of the array type.  */
+      newitype = build_qualified_type (TREE_TYPE (type), TYPE_QUALS (type));
+
         *vecp = build1 (VIEW_CONVERT_EXPR,
-                   build_array_type_nelts (TREE_TYPE (type),
+                   build_array_type_nelts (newitype,
                                             TYPE_VECTOR_SUBPARTS (type)),
                     *vecp);
       }
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index fa346fe01c9..1fe91d16e8e 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -4421,7 +4421,8 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, tree 
t,
     if (!lval
         && TREE_CODE (ary) == VIEW_CONVERT_EXPR
         && VECTOR_TYPE_P (TREE_TYPE (TREE_OPERAND (ary, 0)))
-      && TREE_TYPE (t) == TREE_TYPE (TREE_TYPE (TREE_OPERAND (ary, 0))))
+      && TYPE_MAIN_VARIANT (TREE_TYPE (t))
+       == TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (TREE_OPERAND (ary, 0)))))

Please add parens around the == expression so the formatting is stable.
ok, I will make that change.


With that change, OK for trunk and release branches.

For the GCC 14 branch, should I wait until after the release due to
RC1 going out today and I am not sure this counts as a show stopper
issue.

That's not my call ("all changes to the branch require a RM approval 
now") but I think it can wait for 14.2.

Jason