[Bug c++/108158] New: modification of '...' is not a constant expression

[Bug c++/108158] New: modification of '...' is not a constant expression

[serge.guelton@telecom-bretagne.eu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108158

            Bug ID: 108158
           Summary: modification of '...' is not a constant expression
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: serge.guelton@telecom-bretagne.eu
  Target Milestone: ---

Since version 11.3, gcc fails on compiling a (complex) constant expression from
the frozen[0] library. gcc 11.2 is fine, and so are clang and msvc.

I've reduced the code to the following: https://godbolt.org/z/e5bje7v35
It's still a relatively large piece of code, I failed to reduce it more
significantly, sorry about that. The error message is

```
<source>:458:39:   in 'constexpr' expansion of
'ze_set.frozen::unordered_set<short unsigned int, 3>::lookup<int,
frozen::elsa<short unsigned int> >(4, frozen::elsa<short unsigned int>())'
<source>:453:32:   in 'constexpr' expansion of '((const
frozen::unordered_set<short unsigned int,
3>*)this)->frozen::unordered_set<short unsigned int,
3>::tables_.frozen::bits::pmh_tables<8, frozen::elsa<short unsigned int>
>::lookup<int, frozen::elsa<short unsigned int> >((* & key), (* & hash))'
<source>:458:67:   in 'constexpr' expansion of 'frozen::bits::seed_or_index()'
<source>:458:67: error: modification of 'ze_set' is not a constant expression
  458 | constexpr auto nocount = ze_set.lookup(4, frozen::elsa<uint16_t>());
      | 
```

But I don't see where such a modification would happen, and the fact that older
version of gcc and clang and msvc correctly evaluate that expression makes me
think it could be a GCC bug. Maybe bisecting between 11.2 and 11.3 would help
understanding the origin of the regression?


[0] https://github.com/serge-sans-paille/frozen

[Bug c++/108158] modification of '...' is not a constant expression

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108158

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 54119
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54119&action=edit
testcase

Next text attach the testcase rather than just doing a link to godbolt .

[Bug c++/108158] [11/12/13 Regression] modification of '...' is not a constant expression

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108158

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |rejects-valid
      Known to work|                            |11.2.0
            Summary|modification of '...' is    |[11/12/13 Regression]
                   |not a constant expression   |modification of '...' is
                   |                            |not a constant expression
   Target Milestone|---                         |11.4
      Known to fail|                            |11.3.0, 12.1.0, 13.0

[Bug c++/108158] [11/12/13 Regression] modification of '...' is not a constant expression since r12-2304

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108158

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-12-20
             Status|UNCONFIRMED                 |NEW
           Keywords|needs-bisection             |
     Ever confirmed|0                           |1
            Summary|[11/12/13 Regression]       |[11/12/13 Regression]
                   |modification of '...' is    |modification of '...' is
                   |not a constant expression   |not a constant expression
                   |                            |since r12-2304
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |mpolacek at gcc dot gnu.org

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started to be rejected with r12-2304-ga42f8120442cf3ba25d621bed857b5be19019d0c
aka PR101371 fix, which got later backported to 11 branch in r11-9710.

[Bug c++/108158] [11/12/13 Regression] modification of '...' is not a constant expression since r12-2304

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108158

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |mpolacek at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #3 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Mine then :(

[Bug c++/108158] [11/12/13 Regression] modification of '...' is not a constant expression since r12-2304

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108158

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug c++/108158] [11/12/13 Regression] modification of '...' is not a constant expression since r12-2304

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108158

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marxin at gcc dot gnu.org

--- Comment #4 from Martin Liška <marxin at gcc dot gnu.org> ---
I've just reduced that into:

$ cat pr108158.ii
template <class T, int N> struct carray {
  T data_[N]{};
  constexpr T operator[](long index) const { return data_[index]; }
};
struct seed_or_index {
private:
  long value_ = 0;
};
template <int M> struct pmh_tables {
  carray<seed_or_index, M> first_table_;
  template <typename KeyType, typename HasherType>
  constexpr void lookup(KeyType, HasherType) const {
    first_table_[0];
  }
};
template <int N> struct unordered_set {
  int equal_;
  carray<int, N> keys_;
  pmh_tables<N> tables_;
  constexpr unordered_set() : equal_{} {}
  template <class KeyType, class Hasher>
  constexpr auto lookup(KeyType key, Hasher hash) const {
    tables_.lookup(key, hash);
    return keys_;
  }
};
constexpr unordered_set<3> ze_set;
constexpr auto nocount = ze_set.lookup(4, int());

[Bug c++/108158] [11/12/13 Regression] modification of '...' is not a constant expression since r12-2304

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108158

--- Comment #5 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Candidate fix:

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index be99bec17e7..6718c29b0cd 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -4301,9 +4301,13 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, tree
t,
   if (!SCALAR_TYPE_P (elem_type))
     {
       new_ctx = *ctx;
-      if (ctx->object)
+      if (ctx->object
+         && !same_type_ignoring_top_level_qualifiers_p
+             (TREE_TYPE (t), TREE_TYPE (ctx->object)))
        /* If there was no object, don't add one: it could confuse us
-          into thinking we're modifying a const object.  */
+          into thinking we're modifying a const object.  Similarly, if
+          the types are the same, replacing .object could lead to a
+          failure to evaluate it (c++/108158).  */
        new_ctx.object = t;
       new_ctx.ctor = build_constructor (elem_type, NULL);
       ctx = &new_ctx;

[Bug c++/108158] [11/12/13 Regression] modification of '...' is not a constant expression since r12-2304

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108158

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Marek Polacek <mpolacek@gcc.gnu.org>:

https://gcc.gnu.org/g:27ac6a707e7438c3cec79c24f5d53de79493e2f8

commit r13-5693-g27ac6a707e7438c3cec79c24f5d53de79493e2f8
Author: Marek Polacek <polacek@redhat.com>
Date:   Mon Jan 30 09:02:00 2023 -0500

    c++: wrong error with constexpr array and value-init [PR108158]

    In this test case, we find ourselves evaluating 't' which is
    ((const struct carray *) this)->data_[VIEW_CONVERT_EXPR<long int>(index)]
    in cxx_eval_array_reference.  ctx->object is non-null, a RESULT_DECL, so
    we replace it with 't':

      new_ctx.object = t; // result_decl replaced

    and then we go to cxx_eval_constant_expression to evaluate an
    AGGR_INIT_EXPR, where we end up evaluating an INIT_EXPR (which is in the
    body of the constructor for seed_or_index):

      ((struct seed_or_index *) this)->value_ = NON_LVALUE_EXPR <0>

    whereupon in cxx_eval_store_expression we go to the probe loop
    where the 'this' is evaluated to

      ze_set.tables_.first_table_.data_[0]

    so the 'object' is ze_set, but that isn't in ctx->global->get_value_ptr
    so we fail with a bogus error.  ze_set is not there because it comes
    from a different constexpr context (it's not in cv_cache either).

    The problem started with r12-2304 where I added the new_ctx.object
    replacement.  That was to prevent a type mismatch: the type of 't'
    and ctx.object were different.

    It seems clear that we shouldn't have replaced ctx.object here.
    The cxx_eval_array_reference I mentioned earlier is called from
    cxx_eval_store_expression:
     6257       init = cxx_eval_constant_expression (&new_ctx, init,
vc_prvalue,
     6258                                            non_constant_p,
overflow_p);
    which already created a new context, whose .object we should be
    using unless, for instance, INIT contained a.b and we're evaluating
    the 'a' part, which I think was the case for r12-2304; in that case
    ctx.object has to be something different.

    It no longer seems necessary to replace new_ctx.object (likely due to
    changes in empty class handling).

            PR c++/108158

    gcc/cp/ChangeLog:

            * constexpr.cc (cxx_eval_array_reference): Don't replace
            new_ctx.object.

    gcc/testsuite/ChangeLog:

            * g++.dg/cpp1y/constexpr-108158.C: New test.

[Bug c++/108158] [11/12 Regression] modification of '...' is not a constant expression since r12-2304

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108158

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11/12/13 Regression]       |[11/12 Regression]
                   |modification of '...' is    |modification of '...' is
                   |not a constant expression   |not a constant expression
                   |since r12-2304              |since r12-2304

--- Comment #7 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Fixed on trunk so far.

[Bug c++/108158] [11/12 Regression] modification of '...' is not a constant expression since r12-2304

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108158

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|11.4                        |11.5

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 11.4 is being released, retargeting bugs to GCC 11.5.