[PATCH 4/9] [SFN] introduce statement frontier notes, still disabled

[Jason Merrill]

On Wed, Nov 1, 2017 at 3:13 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> Hi, Jason,
>
> Apologies for the delay in responding; somehow I missed your reply when
> it arrived (probably because I had a cold back then :-), and only saw it
> today.
>
> On Oct 24, 2017, Jason Merrill <jason@redhat.com> wrote:
>
>> On Sat, Sep 30, 2017 at 5:08 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
>>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
>>> index a89ee49..f9209ac 100644
>>> --- a/gcc/cp/constexpr.c
>>> +++ b/gcc/cp/constexpr.c
>>> @@ -306,6 +306,9 @@ build_data_member_initialization (tree t, vec<constructor_elt, va_gc> **vec)
>>> tree_stmt_iterator i;
>>> for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))
>>> {
>>> +         if (TREE_CODE (tsi_stmt (i)) == DEBUG_BEGIN_STMT)
>>> +           /* ??? Can we retain this information somehow?  */
>
>> There's no need; the result of this function isn't used for codegen
>> any more, just checking.
>
>> Why do you need a special case for this?  The existing code should
>> have the same effect.
>
> More than anything, it was to add the comment about retaining the begin
> stmt markers.
>
> Something in me wishes that, in spite of resolving the entire function
> call to a constant expression, we could still express in debug info that
> "we went through these statements", and maybe even single-step through
> them towards the final result.

Sure, but my point is that this function is not involved in evaluating
the call, so there would be no reason for a change here.  Most likely
you'd want something in cxx_eval_store_expression.

>>> @@ -586,6 +590,9 @@ build_constexpr_constructor_member_initializers (tree type, tree body)
>>> tree_stmt_iterator i;
>>> for (i = tsi_start (body); !tsi_end_p (i); tsi_next (&i))
>>> {
>>> +         if (TREE_CODE (tsi_stmt (i)) == DEBUG_BEGIN_STMT)
>>> +           /* ??? Can we retain this information somehow?  */
>
>> Likewise.

>>> @@ -3783,6 +3791,8 @@ cxx_eval_statement_list (const constexpr_ctx *ctx, tree t,
>>> for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i))
>>> {
>>> tree stmt = tsi_stmt (i);
>>> +      if (TREE_CODE (stmt) == DEBUG_BEGIN_STMT)
>>> +       continue;
>
>> Maybe instead of handling this here, add it to the sequence of codes
>> that are returned unchanged in cxx_eval_constant_expression, after
>> PREDICT_EXPR?
>
> That would likely work, but there's a slight risk of codegen changes
> then, if a debug begin stmt happens to be present (for some reason I
> can't imagine) after the statement expression whose result we're
> supposed to use and return: we'd then overwrite the expression value we
> should use with the debug begin stmt.  Defensively, it seemed best to
> avoid that risk, even though in theory it shouldn't ever occur.

> There's another case that's more material, that also involves a debug
> begin stmt at the end of a statement list: that of an empty statement
> list.  It's not expected that cxx_eval_statement_list returns a debug
> begin stmt marker, but that's what it would do with the proposed change.
> Then we'd have to handle that up the call chain.  Maybe it doesn't
> matter, maybe it's no big deal, but by just skipping it there we can
> just not worry about it elsewhere.

I'm skeptical about adding defensive special cases everywhere to deal
with seemingly impossible situations.  If I'm not sure something's
actually impossible, I prefer to add an assert so things break more
loudly.

Jason