public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Marek Polacek <polacek@redhat.com>
To: Jason Merrill <jason@redhat.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: [PATCH v2] c++: error with constexpr operator() [PR107939]
Date: Mon, 6 Mar 2023 17:01:00 -0500	[thread overview]
Message-ID: <ZAZinAZKAELCJ2Sy@redhat.com> (raw)
In-Reply-To: <fd01e962-fc6f-e52a-790c-5fe82b42c82d@redhat.com>

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.

> , 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?

-- >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
-- 
2.39.2


  reply	other threads:[~2023-03-06 22:01 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-03 17:51 [PATCH] " Marek Polacek
2023-03-06 16:12 ` Jason Merrill
2023-03-06 22:01   ` Marek Polacek [this message]
2023-03-07 14:53     ` [PATCH v2] " Jason Merrill
2023-03-07 15:11       ` Marek Polacek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZAZinAZKAELCJ2Sy@redhat.com \
    --to=polacek@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).