[C++,RFH/Patch] PR 52892 (and others)

Hi,

it would be nice to make progress on some constexpr issues, thus I 
started looking more closely into c++/52892, which comes with a compact 
testcase and a useful comparison with a snippet for which we already do 
the right thing (thanks to Daniel). Thus:

constexpr bool is_negative(int x) { return x < 0; }
constexpr bool check(int x, bool (*p)(int)) { return p(x); }
static_assert(check(-2, is_negative), "Error");

vs

constexpr bool is_negative(int x) { return x < 0; }
typedef bool (*Function)(int);
constexpr bool check(int x, Function p) { return p(x); }
static_assert(check(-2, is_negative), "Error");

the difference, for the latter and for more complex cases, is that 
adjust_temp_type calls cp_fold_convert which ends up returning a 
NOP_EXPR (eg, build in fold_convert_loc). This NOP_EXPR eventually 
appears as return value of the cxx_eval_constant_expression at the 
beginning of cxx_eval_call_expression. Of course, there are many 
different ways in which such NOP_EXPR can be returned and, well, if only 
we could look through it, we would find an ADDR_EXPR and all such cases 
(eg, in c++/52282 too) would be easily handled by the rest of 
cxx_eval_call_expression. Today I wondered: at least when we are *sure* 
that we are handling a function pointer, would it be correct to actually 
use STRIP_NOPS on what cxx_eval_constant_expression returns?!? The 
attached passes testing anyway...

Thanks,
Paolo Carlini

On 08/26/2014 12:01 PM, Paolo Carlini wrote:
> the difference, for the latter and for more complex cases, is that
> adjust_temp_type calls cp_fold_convert which ends up returning a
> NOP_EXPR (eg, build in fold_convert_loc).

Perhaps we should address this in adjust_temp_type, either by ignoring 
the conversion to a canonically equivalent type or by directly changing 
the type of the ADDR_EXPR.

Jason

Hi,

On 08/26/2014 08:58 PM, Jason Merrill wrote:
> On 08/26/2014 12:01 PM, Paolo Carlini wrote:
>> the difference, for the latter and for more complex cases, is that
>> adjust_temp_type calls cp_fold_convert which ends up returning a
>> NOP_EXPR (eg, build in fold_convert_loc).
>
> Perhaps we should address this in adjust_temp_type, either by ignoring 
> the conversion to a canonically equivalent type or by directly 
> changing the type of the ADDR_EXPR.
Ok. I didn't tell you in my first message that I already tried changing 
the 'TREE_TYPE (temp) == type' to 'same_type_p (TREE_TYPE (temp), type)' 
and it works for the simple case I posted, but it doesn't in more 
complex cases, involving templates too. Thus something more complex in 
adjust_temp_type, I suppose...

Paolo.

Hi again,

On 08/26/2014 08:58 PM, Jason Merrill wrote:
> On 08/26/2014 12:01 PM, Paolo Carlini wrote:
>> the difference, for the latter and for more complex cases, is that
>> adjust_temp_type calls cp_fold_convert which ends up returning a
>> NOP_EXPR (eg, build in fold_convert_loc).
>
> Perhaps we should address this in adjust_temp_type, either by ignoring 
> the conversion to a canonically equivalent type or by directly 
> changing the type of the ADDR_EXPR.
A big issue is that in some more complex cases, like the oiriginal 
testcase in c++/52892, cxx_eval_call_expression gets a fun which is 
*already* a NOP_EXPR. Because NOP_EXPRs can be generated quite early, 
for example via cp_build_indirect_ref -> decay_conversion -> build_nop 
or later via build_special_member_call -> build_dummy_object.

As soon as a NOP_EXPR infiltrates cxx_eval_call_expression it doesn't 
see anymore the ADDR_EXPR hidden inside and the game is over. Thus, what 
would be the strategy in such cases: NOP_EXPRs should not reach 
cxx_eval_call_expression at all?!? Removed earlier? Any tips about the 
proper place to do that?

Thanks!
Paolo.

.. two additional remarks (maybe obvious, I don't know):
- It also appears to work - for sure for all the tests in c++/52892 + 
the tests in c++/52282 not involving data members (eg, the original one) 
- simply unconditionally calling STRIP_NOPS right after the 
cxx_eval_constant_expression at the beginning of 
cxx_eval_call_expression (or calling it only when, after the fact, we 
know it wraps a function pointer).
- Grepping in semantics.c reveals that in quite a few other places we 
make sure to STRIP_NOPS before checking for ADDR_EXPR, it seems a 
general issue.

Thanks,
Paolo.

On 08/27/2014 04:41 AM, Paolo Carlini wrote:
> .. two additional remarks (maybe obvious, I don't know):
> - It also appears to work - for sure for all the tests in c++/52892 +
> the tests in c++/52282 not involving data members (eg, the original one)
> - simply unconditionally calling STRIP_NOPS right after the
> cxx_eval_constant_expression at the beginning of
> cxx_eval_call_expression (or calling it only when, after the fact, we
> know it wraps a function pointer).
> - Grepping in semantics.c reveals that in quite a few other places we
> make sure to STRIP_NOPS before checking for ADDR_EXPR, it seems a
> general issue.

True.  OK, let's go ahead with your approach.

Jason