Re: [PATCH v2] c++: error with constexpr operator() [PR107939]

[Jason Merrill <jason@redhat.com>]

On 3/6/23 17:01, Marek Polacek wrote:
> On Mon, Mar 06, 2023 at 11:12:56AM -0500, Jason Merrill wrote:
>> On 3/3/23 12:51, Marek Polacek wrote:
>>> Similarly to PR107938, this also started with r11-557, whereby cp_finish_decl
>>> can call check_initializer even in a template for a constexpr initializer.
>>>
>>> Here we are rejecting
>>>
>>>     extern const Q q;
>>>
>>>     template<int>
>>>     constexpr auto p = q(0);
>>>
>>> even though q has a constexpr operator().  It's deemed non-const by
>>> decl_maybe_constant_var_p because even though 'q' is const it is not
>>> of integral/enum type.  I think the fix is for p_c_e to treat q(0) as
>>> potentially-constant, as below.
>>>
>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/12?
>>>
>>> 	PR c++/107939
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	* constexpr.cc (is_constexpr_function_object): New.
>>> 	(potential_constant_expression_1): Treat an object with constexpr
>>> 	operator() as potentially-constant.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	* g++.dg/cpp1y/var-templ74.C: Remove dg-error.
>>> 	* g++.dg/cpp1y/var-templ77.C: New test.
>>> ---
>>>    gcc/cp/constexpr.cc                      | 23 ++++++++++++++++++++++-
>>>    gcc/testsuite/g++.dg/cpp1y/var-templ74.C |  2 +-
>>>    gcc/testsuite/g++.dg/cpp1y/var-templ77.C | 14 ++++++++++++++
>>>    3 files changed, 37 insertions(+), 2 deletions(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ77.C
>>>
>>> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
>>> index acf9847a4d1..7d786f332b4 100644
>>> --- a/gcc/cp/constexpr.cc
>>> +++ b/gcc/cp/constexpr.cc
>>> @@ -8929,6 +8929,24 @@ check_for_return_continue (tree *tp, int *walk_subtrees, void *data)
>>>      return NULL_TREE;
>>>    }
>>> +/* Return true iff TYPE is a class with constexpr operator().  */
>>> +
>>> +static bool
>>> +is_constexpr_function_object (tree type)
>>> +{
>>> +  if (!CLASS_TYPE_P (type))
>>> +    return false;
>>> +
>>> +  for (tree f = TYPE_FIELDS (type); f; f = DECL_CHAIN (f))
>>> +    if (TREE_CODE (f) == FUNCTION_DECL
>>> +	&& DECL_OVERLOADED_OPERATOR_P (f)
>>> +	&& DECL_OVERLOADED_OPERATOR_IS (f, CALL_EXPR)
>>> +	&& DECL_DECLARED_CONSTEXPR_P (f))
>>> +      return true;
>>> +
>>> +  return false;
>>> +}
>>> +
>>>    /* Return true if T denotes a potentially constant expression.  Issue
>>>       diagnostic as appropriate under control of FLAGS.  If WANT_RVAL is true,
>>>       an lvalue-rvalue conversion is implied.  If NOW is true, we want to
>>> @@ -9160,7 +9178,10 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
>>>    	  }
>>>    	else if (fun)
>>>              {
>>> -	    if (RECUR (fun, rval))
>>> +	    if (VAR_P (fun)
>>> +		&& is_constexpr_function_object (TREE_TYPE (fun)))
>>> +	      /* Could be an object with constexpr operator().  */;
>>
>> I guess if fun is not a function pointer, we don't know if we're using it as
>> an lvalue or rvalue
> 
> Presumably the operator function could return this, making it an lvalue?
> I'm not sure I'm really clear on this.

I mean just calling the operator uses the variable as an lvalue, by 
passing its address as 'this'.

>> , so we want to pass 'any' for want_rval, which should
>> make this work;
> 
> Yes, want_rval==false means that p_c_e/VAR_DECL will not issue the
> hard error.
> 
>> I don't think we need to be specific about constexpr op(),
>> as a constexpr conversion operator to fn* could also do the trick.
> 
> Ah, those surrogate classes.  I couldn't reproduce the problem with
> them, though I'm adding a test for it anyway.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

OK, thanks.

> -- >8 --
> Similarly to PR107938, this also started with r11-557, whereby cp_finish_decl
> can call check_initializer even in a template for a constexpr initializer.
> 
> Here we are rejecting
> 
>    extern const Q q;
> 
>    template<int>
>    constexpr auto p = q(0);
> 
> even though q has a constexpr operator().  It's deemed non-const by
> decl_maybe_constant_var_p because even though 'q' is const it is not
> of integral/enum type.
> 
> If fun is not a function pointer, we don't know if we're using it as an
> lvalue or rvalue, so with this patch we pass 'any' for want_rval.  With
> that, p_c_e/VAR_DECL doesn't flat out reject the underlying VAR_DECL.
> 
> 	PR c++/107939
> 
> gcc/cp/ChangeLog:
> 
> 	* constexpr.cc (potential_constant_expression_1) <case CALL_EXPR>: Pass
> 	'any' when recursing on a VAR_DECL and not a pointer to function.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp1y/var-templ74.C: Remove dg-error.
> 	* g++.dg/cpp1y/var-templ77.C: New test.
> ---
>   gcc/cp/constexpr.cc                      |  8 ++++--
>   gcc/testsuite/g++.dg/cpp1y/var-templ74.C |  2 +-
>   gcc/testsuite/g++.dg/cpp1y/var-templ77.C | 32 ++++++++++++++++++++++++
>   3 files changed, 39 insertions(+), 3 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ77.C
> 
> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> index 364695b762c..3079561f2e8 100644
> --- a/gcc/cp/constexpr.cc
> +++ b/gcc/cp/constexpr.cc
> @@ -9179,8 +9179,12 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
>   	  }
>   	else if (fun)
>             {
> -	    if (RECUR (fun, rval))
> -	      /* Might end up being a constant function pointer.  */;
> +	    if (RECUR (fun, FUNCTION_POINTER_TYPE_P (fun) ? rval : any))
> +	      /* Might end up being a constant function pointer.  But it
> +		 could also be a function object with constexpr op(), so
> +		 we pass 'any' so that the underlying VAR_DECL is deemed
> +		 as potentially-constant even though it wasn't declared
> +		 constexpr.  */;
>   	    else
>   	      return false;
>             }
> diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ74.C b/gcc/testsuite/g++.dg/cpp1y/var-templ74.C
> index 4e2e800a6eb..c76a7d949ac 100644
> --- a/gcc/testsuite/g++.dg/cpp1y/var-templ74.C
> +++ b/gcc/testsuite/g++.dg/cpp1y/var-templ74.C
> @@ -9,7 +9,7 @@ struct Q {
>   extern const Q q;
>   
>   template<int>
> -constexpr const Q* p = q(0); // { dg-bogus "not usable" "PR107939" { xfail *-*-* } }
> +constexpr const Q* p = q(0);
>   
>   void
>   g ()
> diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ77.C b/gcc/testsuite/g++.dg/cpp1y/var-templ77.C
> new file mode 100644
> index 00000000000..0c56d70a034
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/var-templ77.C
> @@ -0,0 +1,32 @@
> +// PR c++/107939
> +// { dg-do compile { target c++14 } }
> +
> +struct Q {
> +  struct P {
> +    const Q* p;
> +  };
> +  int n;
> +  constexpr P operator()(int) const { return {this}; }
> +};
> +
> +extern const Q q;
> +template<int>
> +constexpr auto p = q(0);
> +static_assert(p<0>.p == &q, "");
> +
> +constexpr int
> +fn (int)
> +{
> +  return 42;
> +}
> +
> +struct Sur {
> +  using FN = int(int);
> +  constexpr operator FN*() const { return &fn; }
> +};
> +
> +extern const Sur sur;
> +template<int>
> +constexpr int aja = sur (0);
> +static_assert(aja<0> == 42, "");
> +static_assert(sur(1) == 42, "");
> 
> base-commit: 553ff2524f412be4e02e2ffb1a0a3dc3e2280742