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

[Alexandre Oliva <aoliva at redhat dot com>]

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.

But if we all agree we don't want to do that, I'll be glad to drop the
???/FIXME markers to that effect.

>> @@ -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.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer