On Wed, Nov 1, 2017 at 3:13 PM, Alexandre Oliva <aol...@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 <ja...@redhat.com> wrote:
>
>> On Sat, Sep 30, 2017 at 5:08 AM, Alexandre Oliva <aol...@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
