Re: [C++ PATCH] Fix constexpr switch handling (PR c++/77467)

[Jason Merrill <jason@redhat.com>]

On Fri, Sep 16, 2016 at 4:44 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Sep 16, 2016 at 03:51:11PM -0400, Jason Merrill wrote:
>> On Mon, Sep 5, 2016 at 1:11 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > +  /* If body is a statement other than STATEMENT_LIST or BIND_EXPR,
>> > +     it should be skipped.  E.g. switch (a) b = a;  */
>> > +  if (TREE_CODE (body) == STATEMENT_LIST
>> > +      || TREE_CODE (body) == BIND_EXPR)
>>
>> I'm nervous about this optimization for useless code breaking other
>> things that might (one day) wrap a case label; I think I'd prefer to
>> drop the condition.
>
> By droping the condition you mean unconditionally call
>   cxx_eval_constant_expression (ctx, body, false,
>                                 non_constant_p, overflow_p, jump_target);
> ?  That is known not to work, that breaks the
> +constexpr int
> +bar (int x)
> +{
> +  int a = x;
> +  switch (x)
> +    a = x + 1;
> +  return a;
> +}
> handling in the testcase, where body is the MODIFY_EXPR which doesn't have
> the label and thus needs to be skipped.  The problem is that all the logic for
> skipping statements until the label is found is in cxx_eval_statement_list
> only.

Ah, right.

> For STATEMENT_LIST that is called by cxx_eval_constant_expression,
> for BIND_EXPR if we are lucky enough that BIND_EXPR_BODY is a STATEMENT_LIST
> too (otherwise I assume even my patch doesn't fix it, it would need to
> verify that).  If body is some other statement, then it really should be
> skipped, but it isn't, because cxx_eval_constant_expression ignores it.

> I wonder if we e.g. cxx_eval_constant_expression couldn't early in the
> function for if (*jump_target) return immediately unless code is something
> like STATEMENT_LIST or BIND_EXPR with BIND_EXPR_BODY being STATEMENT_LIST,
> or perhaps in the future other construct containing other stmts.

We might assert !jump_target before the call to
cxx_eval_store_expression, to make sure we don't accidentally evaluate
one when we're trying to jump.

> I've beeing thinking about TRY block, but at least on the testcases I've
> tried it has been rejected in constexpr functions, I think one can't branch
> into statement expressions, so that should be fine, OpenMP/OpenACC
> constructs are hopefully also rejected in constexpr, what else?

LOOP_EXPR, COND_EXPR?

Jason