[PATCH] c++: Fix constexpr evaluation of self-modifying CONSTRUCTORs [PR94219]
Jason Merrill
jason@redhat.com
Sat Apr 4 04:11:47 GMT 2020
On 4/3/20 4:57 PM, Patrick Palka wrote:
> On Fri, 3 Apr 2020, Patrick Palka wrote:
>
>> On Fri, 3 Apr 2020, Jason Merrill wrote:
>>
>>> On 4/2/20 2:19 PM, Patrick Palka wrote:
>>>> On Thu, 2 Apr 2020, Patrick Palka wrote:
>>>>
>>>>> This PR reveals that cxx_eval_bare_aggregate and cxx_eval_store_expression
>>>>> do
>>>>> not anticipate that a constructor element's initializer could mutate the
>>>>> underlying CONSTRUCTOR. Evaluation of the initializer could add new
>>>>> elements to
>>>>> the underlying CONSTRUCTOR, thereby potentially invalidating any pointers
>>>>> to
>>>>> or assumptions about the CONSTRUCTOR's elements, and so these routines
>>>>> should be
>>>>> prepared for that.
>>>>>
>>>>> To fix this problem, this patch makes cxx_eval_bare_aggregate and
>>>>> cxx_eval_store_expression recompute the pointer to the constructor_elt's
>>>>> through
>>>>> which we're assigning, after it evaluates the initializer. Care is taken
>>>>> to
>>>>> to make the common case where the initializer does not modify the
>>>>> underlying
>>>>> CONSTRUCTOR as fast as before.
>>>>
>>>> Also, with this patch, I'm not totally sure but I think we might not
>>>> need the special preeval handling in cxx_eval_store_expression anymore.
>>>> I could try to remove it in a subsequent patch.
>>>
>>> I think for C++17 order of evaluation it's better to keep preevaluating when
>>> we can.
>>
>> Sounds good.
>>
>>>
>>>>> gcc/cp/ChangeLog:
>>>>>
>>>>> PR c++/94205
>>>>> PR c++/94219
>>>>> * constexpr.c (get_or_insert_ctor_field): Split out (while adding
>>>>> support for VECTOR_TYPEs, and optimizations for the common case)
>>>>> from ...
>>>>> (cxx_eval_store_expression): ... here. Rename local variable
>>>>> 'changed_active_union_member_p' to 'activated_union_member_p'. Record
>>>>> the sequence of indexes into 'indexes' that yields the subobject we're
>>>>> assigning to. Record the integer offsets of the constructor indexes
>>>>> we're assigning through into 'index_pos_hints'. After evaluating the
>>>>> initializer of the store expression, recompute 'valp' using 'indexes'
>>>>> and 'index_pos_hints' as hints.
>>>>> (cxx_eval_bare_aggregate): Tweak comments. Use
>>>>> get_or_insert_ctor_field
>>>>> to recompute the pointer to the constructor_elt we're assigning
>>>>> through
>>>>> after evaluating each initializer.
>>>>>
>>>>> gcc/testsuite/ChangeLog:
>>>>>
>>>>> PR c++/94205
>>>>> PR c++/94219
>>>>> * g++.dg/cpp1y/constexpr-nsdmi3.C: New test.
>>>>> * g++.dg/cpp1y/constexpr-nsdmi4.C: New test.
>>>>> * g++.dg/cpp1y/constexpr-nsdmi5.C: New test.
>>>>> * g++.dg/cpp1z/lambda-this5.C: New test.
>>>>> ---
>>>>> gcc/cp/constexpr.c | 252 +++++++++++-------
>>>>> gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C | 19 ++
>>>>> gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C | 21 ++
>>>>> gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C | 22 ++
>>>>> gcc/testsuite/g++.dg/cpp1z/lambda-this5.C | 11 +
>>>>> 5 files changed, 228 insertions(+), 97 deletions(-)
>>>>> create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
>>>>> create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
>>>>> create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
>>>>> create mode 100644 gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
>>>>>
>>>>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
>>>>> index 91f0c3ba269..b4173c595f0 100644
>>>>> --- a/gcc/cp/constexpr.c
>>>>> +++ b/gcc/cp/constexpr.c
>>>>> @@ -3151,6 +3151,97 @@ find_array_ctor_elt (tree ary, tree dindex, bool
>>>>> insert)
>>>>> return -1;
>>>>> }
>>>>> +/* Return a pointer to the constructor_elt of CTOR which matches INDEX.
>>>>> If no
>>>>> + matching constructor_elt exists, then add one to CTOR.
>>>>> +
>>>>> + As an optimization, if POS_HINT is non-negative then it is used as a
>>>>> guess
>>>>> + for the (integer) index of the matching constructor_elt within CTOR.
>>>>> */
>>>>> +
>>>>> +static constructor_elt *
>>>>> +get_or_insert_ctor_field (tree ctor, tree index, int pos_hint)
>>>>> +{
>>>>> + tree type = TREE_TYPE (ctor);
>>>>> + if (TREE_CODE (type) == VECTOR_TYPE && index == NULL_TREE)
>>>>> + {
>>>>> + CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (ctor), index, NULL_TREE);
>>>>> + return &CONSTRUCTOR_ELTS (ctor)->last();
>>>>> + }
>>>>> + else if (TREE_CODE (type) == ARRAY_TYPE || TREE_CODE (type) ==
>>>>> VECTOR_TYPE)
>>>>> + {
>>>>> + HOST_WIDE_INT i = find_array_ctor_elt (ctor, index,
>>>>> /*insert*/true);
>>>>> + gcc_assert (i >= 0);
>>>>> + constructor_elt *cep = CONSTRUCTOR_ELT (ctor, i);
>>>>> + gcc_assert (cep->index == NULL_TREE
>>>>> + || TREE_CODE (cep->index) != RANGE_EXPR);
>>>>> + return cep;
>>>>> + }
>>>>> + else
>>>>> + {
>>>>> + gcc_assert (TREE_CODE (index) == FIELD_DECL);
>>>>> +
>>>>> + /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
>>>>> + Usually we meet initializers in that order, but it is
>>>>> + possible for base types to be placed not in program
>>>>> + order. */
>>>>> + tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
>>>>> + unsigned HOST_WIDE_INT idx = 0;
>>>>> + constructor_elt *cep = NULL;
>>>>> +
>>>>> + /* First, check if we're changing the active member of a union. */
>>>>> + if (TREE_CODE (type) == UNION_TYPE && CONSTRUCTOR_NELTS (ctor)
>>>>> + && CONSTRUCTOR_ELT (ctor, 0)->index != index)
>>>>> + vec_safe_truncate (CONSTRUCTOR_ELTS (ctor), 0);
>>>>> + /* Next, check the hint. */
>>>>> + else if (pos_hint >= 0 && (unsigned)pos_hint < CONSTRUCTOR_NELTS
>>>>> (ctor)
>>>>> + && CONSTRUCTOR_ELT (ctor, pos_hint)->index == index)
>>>>> + {
>>>>> + cep = CONSTRUCTOR_ELT (ctor, pos_hint);
>>>>> + goto found;
>>>>> + }
>>>
>>> Don't we want to use pos_hint for all aggregate types?
>>
>> Oops, yes. I moved the hint check up to happen first and
>> unconditionally.
>>
>>>
>>>>> + /* If the hint was wrong, and if the bit offset of INDEX is larger
>>>>> than
>>>>> + that of the last constructor_elt, then we can just immediately append
>>>>> a
>>>>> + new constructor_elt to the end of CTOR. */
>>>>> + else if (CONSTRUCTOR_NELTS (ctor)
>>>>> + && tree_int_cst_compare (bit_position (index),
>>>>> + bit_position (CONSTRUCTOR_ELTS (ctor)
>>>>> + ->last().index)) > 0)
>>>>> + {
>>>>> + idx = CONSTRUCTOR_NELTS (ctor);
>>>>> + goto insert;
>>>>> + }
>>>>> +
>>>>> + /* Otherwise, we need to iterator over CTOR to find or insert INDEX
>>>
>>> s/iterator/iterate/
>>
>> Fixed.
>>
>>>
>>>>> + appropriately. */
>>>>> +
>>>>> + for (; vec_safe_iterate (CONSTRUCTOR_ELTS (ctor), idx, &cep);
>>>>> + idx++, fields = DECL_CHAIN (fields))
>>>>> + {
>>>>> + if (index == cep->index)
>>>>> + goto found;
>>>>> +
>>>>> + /* The field we're initializing must be on the field
>>>>> + list. Look to see if it is present before the
>>>>> + field the current ELT initializes. */
>>>>> + for (; fields != cep->index; fields = DECL_CHAIN (fields))
>>>>> + if (index == fields)
>>>>> + goto insert;
>>>>> + }
>>>>> + /* We fell off the end of the CONSTRUCTOR, so insert a new
>>>>> + entry at the end. */
>>>>> +
>>>>> + insert:
>>>>> + {
>>>>> + constructor_elt ce = { index, NULL_TREE };
>>>>> +
>>>>> + vec_safe_insert (CONSTRUCTOR_ELTS (ctor), idx, ce);
>>>>> + cep = CONSTRUCTOR_ELT (ctor, idx);
>>>>> + }
>>>>> + found:;
>>>>> +
>>>>> + return cep;
>>>>> + }
>>>>> +}
>>>>> +
>>>>> /* Under the control of CTX, issue a detailed diagnostic for
>>>>> an out-of-bounds subscript INDEX into the expression ARRAY. */
>>>>> @@ -3760,14 +3851,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx
>>>>> *ctx, tree t,
>>>>> {
>>>>> tree orig_value = value;
>>>>> init_subob_ctx (ctx, new_ctx, index, value);
>>>>> + int pos_hint = -1;
>>>>> if (new_ctx.ctor != ctx->ctor)
>>>>> - /* If we built a new CONSTRUCTOR, attach it now so that other
>>>>> - initializers can refer to it. */
>>>>> - CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
>>>>> + {
>>>>> + /* If we built a new CONSTRUCTOR, attach it now so that other
>>>>> + initializers can refer to it. */
>>>>> + CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
>>>>> + pos_hint = vec_safe_length (*p) - 1;
>>>>> + }
>>>>> else if (TREE_CODE (type) == UNION_TYPE)
>>>>> - /* Otherwise if we're constructing a union, set the active union
>>>>> member
>>>>> - anyway so that we can later detect if the initializer attempts to
>>>>> - activate another member. */
>>>>> + /* Otherwise if we're constructing a non-aggregate union member, set
>>>>> + the active union member now so that we can later detect and
>>>>> diagnose
>>>>> + if its initializer attempts to activate another member. */
>>>>> CONSTRUCTOR_APPEND_ELT (*p, index, NULL_TREE);
>>>>> tree elt = cxx_eval_constant_expression (&new_ctx, value,
>>>>> lval,
>>>>> @@ -3804,18 +3899,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx,
>>>>> tree t,
>>>>> {
>>>>> if (TREE_CODE (type) == UNION_TYPE
>>>>> && (*p)->last().index != index)
>>>>> - /* The initializer may have erroneously changed the active union
>>>>> - member that we're initializing. */
>>>>> + /* The initializer erroneously changed the active union member
>>>>> that
>>>>> + we're initializing. */
>>>>> gcc_assert (*non_constant_p);
>>>>> - else if (new_ctx.ctor != ctx->ctor
>>>>> - || TREE_CODE (type) == UNION_TYPE)
>>>>> + else
>>>>> {
>>>>> - /* We appended this element above; update the value. */
>>>>> - gcc_assert ((*p)->last().index == index);
>>>>> - (*p)->last().value = elt;
>>>>> + /* The initializer might have mutated the underlying
>>>>> CONSTRUCTOR,
>>>>> + so recompute the location of the target constructer_elt. */
>>>>> + constructor_elt *cep
>>>>> + = get_or_insert_ctor_field (ctx->ctor, index, pos_hint);
>>>>> + cep->value = elt;
>>>>> }
>>>>> - else
>>>>> - CONSTRUCTOR_APPEND_ELT (*p, index, elt);
>>>>> +
>>>>> /* Adding or replacing an element might change the ctor's flags. */
>>>>> TREE_CONSTANT (ctx->ctor) = constant_p;
>>>>> TREE_SIDE_EFFECTS (ctx->ctor) = side_effects_p;
>>>>> @@ -4590,8 +4685,9 @@ cxx_eval_store_expression (const constexpr_ctx *ctx,
>>>>> tree t,
>>>>> type = TREE_TYPE (object);
>>>>> bool no_zero_init = true;
>>>>> - releasing_vec ctors;
>>>>> - bool changed_active_union_member_p = false;
>>>>> + releasing_vec ctors, indexes;
>>>>> + auto_vec<int> index_pos_hints;
>>>>> + bool activated_union_member_p = false;
>>>>> while (!refs->is_empty ())
>>>>> {
>>>>> if (*valp == NULL_TREE)
>>>>> @@ -4632,94 +4728,49 @@ cxx_eval_store_expression (const constexpr_ctx
>>>>> *ctx, tree t,
>>>>> subobjects will also be zero-initialized. */
>>>>> no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
>>>>> - vec_safe_push (ctors, *valp);
>>>>> -
>>>>> enum tree_code code = TREE_CODE (type);
>>>>> type = refs->pop();
>>>>> tree index = refs->pop();
>>>>> - constructor_elt *cep = NULL;
>>>>> - if (code == ARRAY_TYPE)
>>>>> + if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
>>>>> + && CONSTRUCTOR_ELT (*valp, 0)->index != index)
>>>>> {
>>>>> - HOST_WIDE_INT i
>>>>> - = find_array_ctor_elt (*valp, index, /*insert*/true);
>>>>> - gcc_assert (i >= 0);
>>>>> - cep = CONSTRUCTOR_ELT (*valp, i);
>>>>> - gcc_assert (cep->index == NULL_TREE
>>>>> - || TREE_CODE (cep->index) != RANGE_EXPR);
>>>>> - }
>>>>> - else
>>>>> - {
>>>>> - gcc_assert (TREE_CODE (index) == FIELD_DECL);
>>>>> -
>>>>> - /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
>>>>> - Usually we meet initializers in that order, but it is
>>>>> - possible for base types to be placed not in program
>>>>> - order. */
>>>>> - tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
>>>>> - unsigned HOST_WIDE_INT idx;
>>>>> -
>>>>> - if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
>>>>> - && CONSTRUCTOR_ELT (*valp, 0)->index != index)
>>>>> + if (cxx_dialect < cxx2a)
>>>>> {
>>>>> - if (cxx_dialect < cxx2a)
>>>>> - {
>>>>> - if (!ctx->quiet)
>>>>> - error_at (cp_expr_loc_or_input_loc (t),
>>>>> - "change of the active member of a union "
>>>>> - "from %qD to %qD",
>>>>> - CONSTRUCTOR_ELT (*valp, 0)->index,
>>>>> - index);
>>>>> - *non_constant_p = true;
>>>>> - }
>>>>> - else if (TREE_CODE (t) == MODIFY_EXPR
>>>>> - && CONSTRUCTOR_NO_CLEARING (*valp))
>>>>> - {
>>>>> - /* Diagnose changing the active union member while the union
>>>>> - is in the process of being initialized. */
>>>>> - if (!ctx->quiet)
>>>>> - error_at (cp_expr_loc_or_input_loc (t),
>>>>> - "change of the active member of a union "
>>>>> - "from %qD to %qD during initialization",
>>>>> - CONSTRUCTOR_ELT (*valp, 0)->index,
>>>>> - index);
>>>>> - *non_constant_p = true;
>>>>> - }
>>>>> - /* Changing active member. */
>>>>> - vec_safe_truncate (CONSTRUCTOR_ELTS (*valp), 0);
>>>>> - no_zero_init = true;
>>>>> + if (!ctx->quiet)
>>>>> + error_at (cp_expr_loc_or_input_loc (t),
>>>>> + "change of the active member of a union "
>>>>> + "from %qD to %qD",
>>>>> + CONSTRUCTOR_ELT (*valp, 0)->index,
>>>>> + index);
>>>>> + *non_constant_p = true;
>>>>> }
>>>>> -
>>>>> - for (idx = 0;
>>>>> - vec_safe_iterate (CONSTRUCTOR_ELTS (*valp), idx, &cep);
>>>>> - idx++, fields = DECL_CHAIN (fields))
>>>>> + else if (TREE_CODE (t) == MODIFY_EXPR
>>>>> + && CONSTRUCTOR_NO_CLEARING (*valp))
>>>>> {
>>>>> - if (index == cep->index)
>>>>> - goto found;
>>>>> -
>>>>> - /* The field we're initializing must be on the field
>>>>> - list. Look to see if it is present before the
>>>>> - field the current ELT initializes. */
>>>>> - for (; fields != cep->index; fields = DECL_CHAIN (fields))
>>>>> - if (index == fields)
>>>>> - goto insert;
>>>>> + /* Diagnose changing the active union member while the union
>>>>> + is in the process of being initialized. */
>>>>> + if (!ctx->quiet)
>>>>> + error_at (cp_expr_loc_or_input_loc (t),
>>>>> + "change of the active member of a union "
>>>>> + "from %qD to %qD during initialization",
>>>>> + CONSTRUCTOR_ELT (*valp, 0)->index,
>>>>> + index);
>>>>> + *non_constant_p = true;
>>>>> }
>>>>> + no_zero_init = true;
>>>>> + }
>>>>> - /* We fell off the end of the CONSTRUCTOR, so insert a new
>>>>> - entry at the end. */
>>>>> - insert:
>>>>> - {
>>>>> - constructor_elt ce = { index, NULL_TREE };
>>>>> + vec_safe_push (ctors, *valp);
>>>>> + vec_safe_push (indexes, index);
>>>>> - vec_safe_insert (CONSTRUCTOR_ELTS (*valp), idx, ce);
>>>>> - cep = CONSTRUCTOR_ELT (*valp, idx);
>>>>> + constructor_elt *cep
>>>>> + = get_or_insert_ctor_field (*valp, index, /*pos_hint=*/-1);
>>>>> + index_pos_hints.safe_push (cep - CONSTRUCTOR_ELT (*valp, 0));
>>>
>>> I wonder if you want to use valp->begin() rather than CONSTRUCTOR_ELT?
>>
>> Fixed to do CONSTRUCTOR_ELTS (*valp)->begin().
>
> Hmm, but I don't think this is necessary, because after the call to
> get_or_insert_ctor_field, *valp will surely have at least one
> constructor_elt.
No, it's not necessary. The patch is OK either way.
>>
>> Here's the updated patch with the above changes incorporated:
>>
>> -- >8 --
>>
>> gcc/cp/ChangeLog:
>>
>> PR c++/94219
>> PR c++/94205
>> * constexpr.c (get_or_insert_ctor_field): Split out (while adding
>> support for VECTOR_TYPEs, and optimizations for the common case)
>> from ...
>> (cxx_eval_store_expression): ... here. Rename local variable
>> 'changed_active_union_member_p' to 'activated_union_member_p'. Record
>> the sequence of indexes into 'indexes' that yields the subobject we're
>> assigning to. Record the integer offsets of the constructor indexes
>> we're assigning through into 'index_pos_hints'. After evaluating the
>> initializer of the store expression, recompute 'valp' using 'indexes'
>> and using 'index_pos_hints' as hints.
>> (cxx_eval_bare_aggregate): Tweak comments. Use get_or_insert_ctor_field
>> to recompute the pointer to the constructor_elt we're assigning through
>> after evaluating each initializer.
>>
>> gcc/testsuite/ChangeLog:
>>
>> PR c++/94219
>> PR c++/94205
>> * g++.dg/cpp1y/constexpr-nsdmi3.C: New test.
>> * g++.dg/cpp1y/constexpr-nsdmi4.C: New test.
>> * g++.dg/cpp1y/constexpr-nsdmi5.C: New test.
>> * g++.dg/cpp1z/lambda-this5.C: New test.
>> ---
>> gcc/cp/constexpr.c | 250 +++++++++++-------
>> gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C | 19 ++
>> gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C | 21 ++
>> gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C | 22 ++
>> gcc/testsuite/g++.dg/cpp1z/lambda-this5.C | 11 +
>> 5 files changed, 226 insertions(+), 97 deletions(-)
>> create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
>> create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
>> create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
>> create mode 100644 gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
>>
>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
>> index 91f0c3ba269..3c5137bc3b0 100644
>> --- a/gcc/cp/constexpr.c
>> +++ b/gcc/cp/constexpr.c
>> @@ -3151,6 +3151,95 @@ find_array_ctor_elt (tree ary, tree dindex, bool insert)
>> return -1;
>> }
>>
>> +/* Return a pointer to the constructor_elt of CTOR which matches INDEX. If no
>> + matching constructor_elt exists, then add one to CTOR.
>> +
>> + As an optimization, if POS_HINT is non-negative then it is used as a guess
>> + for the (integer) index of the matching constructor_elt within CTOR. */
>> +
>> +static constructor_elt *
>> +get_or_insert_ctor_field (tree ctor, tree index, int pos_hint)
>> +{
>> + /* Check the hint first. */
>> + if (pos_hint >= 0 && (unsigned)pos_hint < CONSTRUCTOR_NELTS (ctor)
>> + && CONSTRUCTOR_ELT (ctor, pos_hint)->index == index)
>> + return CONSTRUCTOR_ELT (ctor, pos_hint);
>> +
>> + tree type = TREE_TYPE (ctor);
>> + if (TREE_CODE (type) == VECTOR_TYPE && index == NULL_TREE)
>> + {
>> + CONSTRUCTOR_APPEND_ELT (CONSTRUCTOR_ELTS (ctor), index, NULL_TREE);
>> + return &CONSTRUCTOR_ELTS (ctor)->last();
>> + }
>> + else if (TREE_CODE (type) == ARRAY_TYPE || TREE_CODE (type) == VECTOR_TYPE)
>> + {
>> + HOST_WIDE_INT i = find_array_ctor_elt (ctor, index, /*insert*/true);
>> + gcc_assert (i >= 0);
>> + constructor_elt *cep = CONSTRUCTOR_ELT (ctor, i);
>> + gcc_assert (cep->index == NULL_TREE
>> + || TREE_CODE (cep->index) != RANGE_EXPR);
>> + return cep;
>> + }
>> + else
>> + {
>> + gcc_assert (TREE_CODE (index) == FIELD_DECL);
>> +
>> + /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
>> + Usually we meet initializers in that order, but it is
>> + possible for base types to be placed not in program
>> + order. */
>> + tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
>> + unsigned HOST_WIDE_INT idx = 0;
>> + constructor_elt *cep = NULL;
>> +
>> + /* Check if we're changing the active member of a union. */
>> + if (TREE_CODE (type) == UNION_TYPE && CONSTRUCTOR_NELTS (ctor)
>> + && CONSTRUCTOR_ELT (ctor, 0)->index != index)
>> + vec_safe_truncate (CONSTRUCTOR_ELTS (ctor), 0);
>> + /* If the bit offset of INDEX is larger than that of the last
>> + constructor_elt, then we can just immediately append a new
>> + constructor_elt to the end of CTOR. */
>> + else if (CONSTRUCTOR_NELTS (ctor)
>> + && tree_int_cst_compare (bit_position (index),
>> + bit_position (CONSTRUCTOR_ELTS (ctor)
>> + ->last().index)) > 0)
>> + {
>> + idx = CONSTRUCTOR_NELTS (ctor);
>> + goto insert;
>> + }
>> +
>> + /* Otherwise, we need to iterate over CTOR to find or insert INDEX
>> + appropriately. */
>> +
>> + for (; vec_safe_iterate (CONSTRUCTOR_ELTS (ctor), idx, &cep);
>> + idx++, fields = DECL_CHAIN (fields))
>> + {
>> + if (index == cep->index)
>> + goto found;
>> +
>> + /* The field we're initializing must be on the field
>> + list. Look to see if it is present before the
>> + field the current ELT initializes. */
>> + for (; fields != cep->index; fields = DECL_CHAIN (fields))
>> + if (index == fields)
>> + goto insert;
>> + }
>> + /* We fell off the end of the CONSTRUCTOR, so insert a new
>> + entry at the end. */
>> +
>> + insert:
>> + {
>> + constructor_elt ce = { index, NULL_TREE };
>> +
>> + vec_safe_insert (CONSTRUCTOR_ELTS (ctor), idx, ce);
>> + cep = CONSTRUCTOR_ELT (ctor, idx);
>> + }
>> + found:;
>> +
>> + return cep;
>> + }
>> +}
>> +
>> /* Under the control of CTX, issue a detailed diagnostic for
>> an out-of-bounds subscript INDEX into the expression ARRAY. */
>>
>> @@ -3760,14 +3849,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree t,
>> {
>> tree orig_value = value;
>> init_subob_ctx (ctx, new_ctx, index, value);
>> + int pos_hint = -1;
>> if (new_ctx.ctor != ctx->ctor)
>> - /* If we built a new CONSTRUCTOR, attach it now so that other
>> - initializers can refer to it. */
>> - CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
>> + {
>> + /* If we built a new CONSTRUCTOR, attach it now so that other
>> + initializers can refer to it. */
>> + CONSTRUCTOR_APPEND_ELT (*p, index, new_ctx.ctor);
>> + pos_hint = vec_safe_length (*p) - 1;
>> + }
>> else if (TREE_CODE (type) == UNION_TYPE)
>> - /* Otherwise if we're constructing a union, set the active union member
>> - anyway so that we can later detect if the initializer attempts to
>> - activate another member. */
>> + /* Otherwise if we're constructing a non-aggregate union member, set
>> + the active union member now so that we can later detect and diagnose
>> + if its initializer attempts to activate another member. */
>> CONSTRUCTOR_APPEND_ELT (*p, index, NULL_TREE);
>> tree elt = cxx_eval_constant_expression (&new_ctx, value,
>> lval,
>> @@ -3804,18 +3897,18 @@ cxx_eval_bare_aggregate (const constexpr_ctx *ctx, tree t,
>> {
>> if (TREE_CODE (type) == UNION_TYPE
>> && (*p)->last().index != index)
>> - /* The initializer may have erroneously changed the active union
>> - member that we're initializing. */
>> + /* The initializer erroneously changed the active union member that
>> + we're initializing. */
>> gcc_assert (*non_constant_p);
>> - else if (new_ctx.ctor != ctx->ctor
>> - || TREE_CODE (type) == UNION_TYPE)
>> + else
>> {
>> - /* We appended this element above; update the value. */
>> - gcc_assert ((*p)->last().index == index);
>> - (*p)->last().value = elt;
>> + /* The initializer might have mutated the underlying CONSTRUCTOR,
>> + so recompute the location of the target constructer_elt. */
>> + constructor_elt *cep
>> + = get_or_insert_ctor_field (ctx->ctor, index, pos_hint);
>> + cep->value = elt;
>> }
>> - else
>> - CONSTRUCTOR_APPEND_ELT (*p, index, elt);
>> +
>> /* Adding or replacing an element might change the ctor's flags. */
>> TREE_CONSTANT (ctx->ctor) = constant_p;
>> TREE_SIDE_EFFECTS (ctx->ctor) = side_effects_p;
>> @@ -4590,8 +4683,9 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>> type = TREE_TYPE (object);
>> bool no_zero_init = true;
>>
>> - releasing_vec ctors;
>> - bool changed_active_union_member_p = false;
>> + releasing_vec ctors, indexes;
>> + auto_vec<int> index_pos_hints;
>> + bool activated_union_member_p = false;
>> while (!refs->is_empty ())
>> {
>> if (*valp == NULL_TREE)
>> @@ -4632,94 +4726,49 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>> subobjects will also be zero-initialized. */
>> no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
>>
>> - vec_safe_push (ctors, *valp);
>> -
>> enum tree_code code = TREE_CODE (type);
>> type = refs->pop();
>> tree index = refs->pop();
>>
>> - constructor_elt *cep = NULL;
>> - if (code == ARRAY_TYPE)
>> + if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
>> + && CONSTRUCTOR_ELT (*valp, 0)->index != index)
>> {
>> - HOST_WIDE_INT i
>> - = find_array_ctor_elt (*valp, index, /*insert*/true);
>> - gcc_assert (i >= 0);
>> - cep = CONSTRUCTOR_ELT (*valp, i);
>> - gcc_assert (cep->index == NULL_TREE
>> - || TREE_CODE (cep->index) != RANGE_EXPR);
>> - }
>> - else
>> - {
>> - gcc_assert (TREE_CODE (index) == FIELD_DECL);
>> -
>> - /* We must keep the CONSTRUCTOR's ELTS in FIELD order.
>> - Usually we meet initializers in that order, but it is
>> - possible for base types to be placed not in program
>> - order. */
>> - tree fields = TYPE_FIELDS (DECL_CONTEXT (index));
>> - unsigned HOST_WIDE_INT idx;
>> -
>> - if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp)
>> - && CONSTRUCTOR_ELT (*valp, 0)->index != index)
>> + if (cxx_dialect < cxx2a)
>> {
>> - if (cxx_dialect < cxx2a)
>> - {
>> - if (!ctx->quiet)
>> - error_at (cp_expr_loc_or_input_loc (t),
>> - "change of the active member of a union "
>> - "from %qD to %qD",
>> - CONSTRUCTOR_ELT (*valp, 0)->index,
>> - index);
>> - *non_constant_p = true;
>> - }
>> - else if (TREE_CODE (t) == MODIFY_EXPR
>> - && CONSTRUCTOR_NO_CLEARING (*valp))
>> - {
>> - /* Diagnose changing the active union member while the union
>> - is in the process of being initialized. */
>> - if (!ctx->quiet)
>> - error_at (cp_expr_loc_or_input_loc (t),
>> - "change of the active member of a union "
>> - "from %qD to %qD during initialization",
>> - CONSTRUCTOR_ELT (*valp, 0)->index,
>> - index);
>> - *non_constant_p = true;
>> - }
>> - /* Changing active member. */
>> - vec_safe_truncate (CONSTRUCTOR_ELTS (*valp), 0);
>> - no_zero_init = true;
>> + if (!ctx->quiet)
>> + error_at (cp_expr_loc_or_input_loc (t),
>> + "change of the active member of a union "
>> + "from %qD to %qD",
>> + CONSTRUCTOR_ELT (*valp, 0)->index,
>> + index);
>> + *non_constant_p = true;
>> }
>> -
>> - for (idx = 0;
>> - vec_safe_iterate (CONSTRUCTOR_ELTS (*valp), idx, &cep);
>> - idx++, fields = DECL_CHAIN (fields))
>> + else if (TREE_CODE (t) == MODIFY_EXPR
>> + && CONSTRUCTOR_NO_CLEARING (*valp))
>> {
>> - if (index == cep->index)
>> - goto found;
>> -
>> - /* The field we're initializing must be on the field
>> - list. Look to see if it is present before the
>> - field the current ELT initializes. */
>> - for (; fields != cep->index; fields = DECL_CHAIN (fields))
>> - if (index == fields)
>> - goto insert;
>> + /* Diagnose changing the active union member while the union
>> + is in the process of being initialized. */
>> + if (!ctx->quiet)
>> + error_at (cp_expr_loc_or_input_loc (t),
>> + "change of the active member of a union "
>> + "from %qD to %qD during initialization",
>> + CONSTRUCTOR_ELT (*valp, 0)->index,
>> + index);
>> + *non_constant_p = true;
>> }
>> + no_zero_init = true;
>> + }
>>
>> - /* We fell off the end of the CONSTRUCTOR, so insert a new
>> - entry at the end. */
>> - insert:
>> - {
>> - constructor_elt ce = { index, NULL_TREE };
>> + vec_safe_push (ctors, *valp);
>> + vec_safe_push (indexes, index);
>>
>> - vec_safe_insert (CONSTRUCTOR_ELTS (*valp), idx, ce);
>> - cep = CONSTRUCTOR_ELT (*valp, idx);
>> + constructor_elt *cep
>> + = get_or_insert_ctor_field (*valp, index, /*pos_hint=*/-1);
>> + index_pos_hints.safe_push (cep - CONSTRUCTOR_ELTS (*valp)->begin());
>> +
>> + if (code == UNION_TYPE)
>> + activated_union_member_p = true;
>>
>> - if (code == UNION_TYPE)
>> - /* Record that we've changed an active union member. */
>> - changed_active_union_member_p = true;
>> - }
>> - found:;
>> - }
>> valp = &cep->value;
>> }
>>
>> @@ -4800,9 +4849,16 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>> init = tinit;
>> init = cxx_eval_constant_expression (&new_ctx, init, false,
>> non_constant_p, overflow_p);
>> - if (ctors->is_empty())
>> - /* The hash table might have moved since the get earlier. */
>> - valp = ctx->global->values.get (object);
>> + /* The hash table might have moved since the get earlier, and the
>> + initializer might have mutated the underlying CONSTRUCTORs, so we must
>> + recompute VALP. */
>> + valp = ctx->global->values.get (object);
>> + for (unsigned i = 0; i < vec_safe_length (indexes); i++)
>> + {
>> + constructor_elt *cep
>> + = get_or_insert_ctor_field (*valp, indexes[i], index_pos_hints[i]);
>> + valp = &cep->value;
>> + }
>> }
>>
>> /* Don't share a CONSTRUCTOR that might be changed later. */
>> @@ -4847,7 +4903,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
>> unsigned i;
>> bool c = TREE_CONSTANT (init);
>> bool s = TREE_SIDE_EFFECTS (init);
>> - if (!c || s || changed_active_union_member_p)
>> + if (!c || s || activated_union_member_p)
>> FOR_EACH_VEC_ELT (*ctors, i, elt)
>> {
>> if (!c)
>> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
>> new file mode 100644
>> index 00000000000..0f91e93ca3f
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi3.C
>> @@ -0,0 +1,19 @@
>> +// PR c++/94205
>> +// { dg-do compile { target c++14 } }
>> +
>> +struct S
>> +{
>> + struct A
>> + {
>> + S *p;
>> + constexpr A(S* p): p(p) {}
>> + constexpr operator int() { p->a = 5; return 6; }
>> + };
>> + int a = A(this);
>> +};
>> +
>> +
>> +constexpr S s = {};
>> +
>> +#define SA(X) static_assert((X),#X)
>> +SA(s.a == 6);
>> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
>> new file mode 100644
>> index 00000000000..0ceef1bb29f
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi4.C
>> @@ -0,0 +1,21 @@
>> +// PR c++/94219
>> +// { dg-do compile { target c++14 } }
>> +
>> +struct A { long x; };
>> +
>> +struct U;
>> +constexpr A foo(U *up);
>> +
>> +struct U {
>> + A a = foo(this); int y;
>> +};
>> +
>> +constexpr A foo(U *up) {
>> + up->y = 11;
>> + return {42};
>> +}
>> +
>> +extern constexpr U u = {};
>> +
>> +static_assert(u.y == 11, "");
>> +static_assert(u.a.x == 42, "");
>> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
>> new file mode 100644
>> index 00000000000..59e7a10d6e8
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi5.C
>> @@ -0,0 +1,22 @@
>> +// PR c++/94219
>> +// { dg-do compile { target c++14 } }
>> +
>> +struct A { long x; };
>> +
>> +struct U;
>> +constexpr A foo(U *up);
>> +
>> +struct U {
>> + U() = default;
>> + int y; A a = foo(this);
>> +};
>> +
>> +constexpr A foo(U *up) {
>> + up->y = 11;
>> + return {42};
>> +}
>> +
>> +extern constexpr U u = {};
>> +
>> +static_assert(u.y == 11, "");
>> +static_assert(u.a.x == 42, "");
>> diff --git a/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C b/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
>> new file mode 100644
>> index 00000000000..c6d44d7fd0b
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/cpp1z/lambda-this5.C
>> @@ -0,0 +1,11 @@
>> +// PR c++/94205
>> +// { dg-do compile { target c++17 } }
>> +
>> +struct S
>> +{
>> + int a = [this] { this->a = 5; return 6; } ();
>> +};
>> +
>> +constexpr S s = {};
>> +
>> +static_assert(s.a == 6);
>> --
>> 2.26.0.106.g9fadedd637
>>
>>
>
More information about the Gcc-patches
mailing list