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

[Marek Polacek <polacek@redhat.com>]

On Tue, Mar 07, 2023 at 09:53:28AM -0500, Jason Merrill wrote:
> 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'.

Ah yeah, right.  Unless there's the && ref-qual etc.
 
> > > , 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.

Thanks.

Marek