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

2016-09-28 Thread Jason Merrill

OK, thanks.

Jason


[C++ PATCH] Fix constexpr switch handling (PR c++/77467, take 2)

2016-09-20 Thread Jakub Jelinek
On Mon, Sep 19, 2016 at 02:34:17PM -0400, Jason Merrill wrote:
> > 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 done that.

> > 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?

Looking at those and adding further testcases revealed lots of other issues,
which the following patch attempts to deal with.

One is that handling the CASE_LABEL_EXPR or LABEL_EXPR only inside of
cxx_eval_statement_list doesn't really work well, because as the testcase
shows, for labeled null statements inside say if then block or else block
there is no STATEMENT_LIST wrapping it up.

Another is that the default: label can be arbitrarily nested (in inner
STATEMENT_LIST, or LOOP_EXPR body, or COND_EXPR then or else block, so
trying to handle it in cxx_eval_statement_list also works only in the
simplest switch case, we really need to process (with initial skipping mode)
the whole SWITCH_EXPR body and if we get through everything and (as
optimization) determine there has been a default: label, restart processing
it again with a flag to satisfy switches (jump_target) with the default:
label of the current switch.

Another thing is that the skipping of statements in cxx_eval_statement_list
didn't really work correctly for COND_EXPR/LOOP_EXPR, so the patch pretty
much removes all the skip/label/etc. handling from cxx_eval_statement_list.

And, lastly the handling of LOOP_EXPR and COND_EXPR when initially skipping
had to be changed.  For COND_EXPR, we don't want to evaluate the condition
in that case (one can't goto into the condition expression), and need to
process then block with skipping and, if it isn't skipped at its end, not
process else block, otherwise process it.  And for LOOP_EXPR, if still
skipping we'd need not to iterate again on the body, as the whole loop has
been bypassed in that case.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-09-20  Jakub Jelinek  

PR c++/77467
* constexpr.c (enum constexpr_switch_state): New.
(struct constexpr_ctx): Add css_state field.
(label_matches): Add CTX and STMT arguments, remove I and
DEFAULT_LABEL.  For CASE_LABEL_EXPR assert ctx->css_state != NULL,
handle default labels according to css_state.
(cxx_eval_statement_list): Remove statement skipping, label_matches
and default_label handling code.
(cxx_eval_loop_expr): Exit after first iteration even if
switches (jump_target).
(cxx_eval_switch_expr): Set up css_state field in ctx, if default
label has been seen in the body, but no cases matched, evaluate
the body second time.
(cxx_eval_constant_expression): Handle stmt skipping and label_matches
here.  Handle PREDICT_EXPR.  For MODIFY_EXPR or INIT_EXPR, assert
statement is not skipped.  For COND_EXPR during skipping, don't
evaluate condition, just the then block and if still skipping at the
end also the else block.
(cxx_eval_outermost_constant_expr): Adjust constexpr_ctx initializer.
(is_sub_constant_expr): Likewise.

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

--- gcc/cp/constexpr.c.jj   2016-09-19 16:35:32.835574406 +0200
+++ gcc/cp/constexpr.c  2016-09-20 12:51:48.270305879 +0200
@@ -900,6 +900,18 @@ struct constexpr_call_hasher : ggc_ptr_h
   static bool equal (constexpr_call *, constexpr_call *);
 };
 
+enum constexpr_switch_state {
+  /* Used when processing a switch for the first time by cxx_eval_switch_expr
+ and default: label for that switch has not been seen yet.  */
+  css_default_not_seen,
+  /* Used when processing a switch for the first time by cxx_eval_switch_expr
+ and default: label for that switch has been seen already.  */
+  css_default_seen,
+  /* Used when processing a switch for the second time by
+ cxx_eval_switch_expr, where default: label should match.  */
+  css_default_processing
+};
+
 /* The constexp