C++ PATCH to fix an ICE on invalid with OVERLOADs (PR c++/84854)

[Jason Merrill]

On Thu, Mar 15, 2018 at 7:49 AM, Marek Polacek <polacek@redhat.com> wrote:
> On Wed, Mar 14, 2018 at 02:06:36PM -0400, Jason Merrill wrote:
>> On Wed, Mar 14, 2018 at 11:59 AM, Marek Polacek <polacek@redhat.com> wrote:
>> > cxx_constant_value doesn't understand template codes, and neither it
>> > understands OVERLOADs, so if we pass an OVERLOAD to it, we crash.  Here
>> > instantiate_non_dependent_expr got an OVERLOAD, but since it calls
>> > is_nondependent_constant_expression which checks type_unknown_p, it left the
>> > expression as it was.  We can't use is_nondependent_constant_expression in
>> > finish_if_stmt_cond because i_n_c_e checks is_constant_expression and that is
>> > not suitable here; we'd miss diagnostics.  So I did the following; I think we
>> > should reject the testcase with an error.
>> >
>> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
>> >
>> > 2018-03-14  Marek Polacek  <polacek@redhat.com>
>> >
>> >         PR c++/84854
>> >         * semantics.c (finish_if_stmt_cond): Give error if the condition
>> >         is an overloaded function with no contextual type information.
>> >
>> >         * g++.dg/cpp1z/constexpr-if15.C: New test.
>> >
>> > diff --git gcc/cp/semantics.c gcc/cp/semantics.c
>> > index fdf37bea770..a056e9445e9 100644
>> > --- gcc/cp/semantics.c
>> > +++ gcc/cp/semantics.c
>> > @@ -735,8 +735,16 @@ finish_if_stmt_cond (tree cond, tree if_stmt)
>> >        && require_constant_expression (cond)
>> >        && !value_dependent_expression_p (cond))
>> >      {
>> > -      cond = instantiate_non_dependent_expr (cond);
>> > -      cond = cxx_constant_value (cond, NULL_TREE);
>> > +      if (type_unknown_p (cond))
>> > +       {
>> > +         cxx_incomplete_type_error (cond, TREE_TYPE (cond));
>> > +         cond = error_mark_node;
>>
>> I think I'd prefer to skip this block when type_unknown_p, and leave
>> error handling up to the code shared with regular if.
>
> Like this?

Yes, thanks.

> It was my first version, but I thought we wanted the error;

Getting an error is an improvement, but it should apply to
non-constexpr if as well, so checking in maybe_convert_cond would be
better.  Actually, if we deal with unknown type there, we shouldn't
need this patch at all.

...except I also notice that since maybe_convert_cond doesn't do any
conversion in a template, we're trying to extract the constant value
without first converting to bool, which breaks this testcase:

struct A
{
  constexpr operator bool () { return true; }
  int i;
};

A a;

template <class T> void f()
{
  constexpr bool b = a;  // works
  if constexpr (a) { }   // breaks
}

int main()
{
  f<int>();
}

A simple fix would be to replace your type_unknown_p check here with a
comparison to boolean_type_node, so we wait until instantiation time
to require a constant value.

Better would be to actually do the conversion.  Perhaps this could
share code (for converting and getting a constant value) with
finish_static_assert.

Jason