c++: Handle CONSTRUCTORs without indexes in find_array_ctor_elt [PR93549]
authorJakub Jelinek <jakub@redhat.com>
Sat, 8 Feb 2020 14:11:28 +0000 (15:11 +0100)
committerJakub Jelinek <jakub@redhat.com>
Sat, 8 Feb 2020 14:11:28 +0000 (15:11 +0100)
My change
* typeck2.c (store_init_value): Don't call cp_fully_fold_init on
initializers of automatic non-constexpr variables in constexpr
functions.
-  value = cp_fully_fold_init (value);
+  /* Don't fold initializers of automatic variables in constexpr functions,
+     that might fold away something that needs to be diagnosed at constexpr
+     evaluation time.  */
+  if (!current_function_decl
+      || !DECL_DECLARED_CONSTEXPR_P (current_function_decl)
+      || TREE_STATIC (decl))
+    value = cp_fully_fold_init (value);
from the constexpr new change apparently broke the following testcase.
When handling COND_EXPR, we build_vector_from_val, however as the argument we
pass to it is not an INTEGER_CST/REAL_CST, but that wrapped in a
NON_LVALUE_EXPR location wrapper, we end up with a CONSTRUCTOR and as it is
middle-end that builds it, it doesn't bother with indexes.  The
cp_fully_fold_init call used to fold it into VECTOR_CST in the past, but as
we intentionally don't invoke it anymore as it might fold away something
that needs to be diagnosed during constexpr evaluation, we end up evaluating
ARRAY_REF into the index-less CONSTRUCTOR.  The following patch fixes the
ICE by teaching find_array_ctor_elt to handle CONSTRUCTORs without indexes
(that itself could be still very efficient) and CONSTRUCTORs with some
indexes present and others missing (the rules are that if the index on the
first element is missing, then it is the array's lowest index (in C/C++ 0)
and if other indexes are missing, they are the index of the previous element
+ 1).

Here is a new version, which assumes CONSTRUCTORs with all or none indexes
and for CONSTRUCTORs without indexes performs the verification for
flag_checking directly in find_array_ctor_elt.  For CONSTRUCTORs with
indexes, it doesn't do the verification of all elts, because some CONSTRUCTORs
can be large, and it "verifies" only what it really needs - if all elts
touched during the binary search have indexes, that is actually all we care
about because we are sure we found the right elt.  It is just if we see a
missing index we need assurance that all are missing to be able to directly
access it.

The assumption then simplifies the patch, for no index CONSTRUCTORs we can
use direct access like for CONSTRUCTORs where last elt index is equal to the
elt position.  If we append right after the last elt, we just should clear
the index so that we don't violate the assumption, and if we need a gap
between the elts and the elt to be added, we need to add indexes.

2020-02-08  Jakub Jelinek  <jakub@redhat.com>

PR c++/93549
* constexpr.c (find_array_ctor_elt): If last element has no index,
for flag_checking verify all elts have no index.  If i is within the
elts, return it directly, if it is right after the last elt, append
if NULL index, otherwise force indexes on all elts.
(cxx_eval_store_expression): Allow cep->index to be NULL.

* g++.dg/ext/constexpr-pr93549.C: New test.

gcc/cp/ChangeLog
gcc/cp/constexpr.c
gcc/testsuite/ChangeLog
gcc/testsuite/g++.dg/ext/constexpr-pr93549.C [new file with mode: 0644]

index f1fe7747f394646598ebc1d322715fa3f6facaae..9205b200cd6e38c02bd2dc818ce75d6c31289af1 100644 (file)
@@ -1,3 +1,12 @@
+2020-02-08  Jakub Jelinek  <jakub@redhat.com>
+
+       PR c++/93549
+       * constexpr.c (find_array_ctor_elt): If last element has no index,
+       for flag_checking verify all elts have no index.  If i is within the
+       elts, return it directly, if it is right after the last elt, append
+       if NULL index, otherwise force indexes on all elts.
+       (cxx_eval_store_expression): Allow cep->index to be NULL.
+
 2020-02-07  Marek Polacek  <polacek@redhat.com>
 
        PR c++/92947 - Paren init of aggregates in unevaluated context.
index 8a02c6e07137e293cf45a71b1f1f67d48e57a7be..aa47ded52c0b958c264853bdd78df9b294f5c76a 100644 (file)
@@ -3028,8 +3028,32 @@ find_array_ctor_elt (tree ary, tree dindex, bool insert)
   if (end > 0)
     {
       tree cindex = (*elts)[end - 1].index;
-      if (TREE_CODE (cindex) == INTEGER_CST
-         && compare_tree_int (cindex, end - 1) == 0)
+      if (cindex == NULL_TREE)
+       {
+         /* Verify that if the last index is missing, all indexes
+            are missing.  */
+         if (flag_checking)
+           for (unsigned int j = 0; j < len - 1; ++j)
+             gcc_assert ((*elts)[j].index == NULL_TREE);
+         if (i < end)
+           return i;
+         else
+           {
+             begin = end;
+             if (i == end)
+               /* If the element is to be added right at the end,
+                  make sure it is added with cleared index too.  */
+               dindex = NULL_TREE;
+             else if (insert)
+               /* Otherwise, in order not to break the assumption
+                  that CONSTRUCTOR either has all indexes or none,
+                  we need to add indexes to all elements.  */
+               for (unsigned int j = 0; j < len; ++j)
+                 (*elts)[j].index = build_int_cst (TREE_TYPE (dindex), j);
+           }
+       }
+      else if (TREE_CODE (cindex) == INTEGER_CST
+              && compare_tree_int (cindex, end - 1) == 0)
        {
          if (i < end)
            return i;
@@ -4551,7 +4575,8 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
            = find_array_ctor_elt (*valp, index, /*insert*/true);
          gcc_assert (i >= 0);
          cep = CONSTRUCTOR_ELT (*valp, i);
-         gcc_assert (TREE_CODE (cep->index) != RANGE_EXPR);
+         gcc_assert (cep->index == NULL_TREE
+                     || TREE_CODE (cep->index) != RANGE_EXPR);
        }
       else
        {
index 149996686b5057d6327f59bdd23c9cec4dffc864..cfe4197c751c48e2e3ed36a77f7360e8e95d2c3e 100644 (file)
@@ -1,3 +1,8 @@
+2020-02-08  Jakub Jelinek  <jakub@redhat.com>
+
+       PR c++/93549
+       * g++.dg/ext/constexpr-pr93549.C: New test.
+
 2020-02-08  Uroš Bizjak  <ubizjak@gmail.com>
            Jakub Jelinek  <jakub@redhat.com>
 
diff --git a/gcc/testsuite/g++.dg/ext/constexpr-pr93549.C b/gcc/testsuite/g++.dg/ext/constexpr-pr93549.C
new file mode 100644 (file)
index 0000000..eaa232d
--- /dev/null
@@ -0,0 +1,26 @@
+// PR c++/93549
+// { dg-do compile { target c++17 } }
+// { dg-options "-O2 -Wno-psabi -w" }
+
+struct simd {
+  using shortx8 [[gnu::vector_size(16)]] = short;
+  shortx8 data;
+  constexpr simd (short x) : data{x, x, x, x, x, x, x, x} {}
+  constexpr friend unsigned operator== (simd lhs, simd rhs)
+  {
+    shortx8 tmp = lhs.data == rhs.data;
+    using ushort = unsigned short;
+    auto bools = tmp ? ushort(1) : ushort(0);
+    unsigned bits = 0;
+    for (int i = 0; i < 8; ++i)
+      bits |= bools[i] << i;
+    return bits;
+  }
+};
+
+auto
+foo ()
+{
+  constexpr auto tmp = simd(1) == simd(2);
+  return tmp;
+}