public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Patrick Palka <ppalka@redhat.com>
To: gcc-patches@gcc.gnu.org
Cc: jason@redhat.com, Patrick Palka <ppalka@redhat.com>
Subject: [PATCH] c++: constantness of local var in constexpr fn [PR111703, PR112269]
Date: Tue, 31 Oct 2023 14:17:26 -0400	[thread overview]
Message-ID: <20231031181726.3944801-1-ppalka@redhat.com> (raw)

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?  Does it look OK for release branches as well for sake of PR111703?

-- >8 --

potential_constant_expression was incorrectly treating most local
variables from a constexpr function as (potentially) constant because it
wasn't considering the 'now' parameter.  This patch fixes this by
relaxing some var_in_maybe_constexpr_fn checks accordingly, which turns
out to partially fix two recently reported regressions:

PR111703 is a regression caused by r11-550-gf65a3299a521a4 for
restricting constexpr evaluation during warning-dependent folding.
The mechanism is intended to restrict only constant evaluation of the
instantiated non-dependent expression, but it also ends up restricting
constant evaluation (as part of satisfaction) during instantiation of
the expression, in particular when resolving the ck_rvalue conversion of
the 'x' argument into a copy constructor call.  This seems like a bug in
the mechanism[1], though I don't know if we want to refine the mechanism
or get rid of it completely since the original testcases which motivated
the mechanism are fixed more simply by r13-1225-gb00b95198e6720.  In any
case, this patch partially fixes this by making us correctly treat 'x'
and therefore 'f(x)' in the below testcase as non-constant, which
prevents the problematic warning-dependent folding from occurring at
all.  If this bug crops up again then I figure we could decide what to
do with the mechanism then.

PR112269 is caused by r14-4796-g3e3d73ed5e85e7 for merging tsubst_copy
into tsubst_copy_and_build.  tsubst_copy used to exit early when 'args'
was empty, behavior which that commit deliberately didn't preserve.
This early exit masked the fact that COMPLEX_EXPR wasn't handled by
tsubst at all, and is a tree code that apparently we could see during
warning-dependent folding on some targets.  A complete fix is to add
handling for this tree code in tsubst_expr, but this patch should fix
the reported testsuite failures since the situations where COMPLEX_EXPR
crops up in <complex> turn out to not be constant expressions in the
first place after this patch.

[1]: The mechanism incorrectly assumes that instantiation of the
non-dependent expression shouldn't induce any template instantiation
since ahead of time checking of the expression should've already induced
whatever template instantiation was needed, but in this case although
overload resolution was performed ahead of time, a ck_rvalue conversion
gets resolved to a copy constructor call only at instantiation time.

	PR c++/111703

gcc/cp/ChangeLog:

	* constexpr.cc (potential_constant_expression_1) <case VAR_DECL>:
	Only consider var_in_maybe_constexpr_fn if 'now' is false.
	<case INDIRECT_REF>: Likewise.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/concepts-fn8.C: New test.
---
 gcc/cp/constexpr.cc                       |  4 ++--
 gcc/testsuite/g++.dg/cpp2a/concepts-fn8.C | 24 +++++++++++++++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-fn8.C

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index c05760e6789..8a6b210144a 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -9623,7 +9623,7 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
 	  return RECUR (DECL_VALUE_EXPR (t), rval);
 	}
       if (want_rval
-	  && !var_in_maybe_constexpr_fn (t)
+	  && (now || !var_in_maybe_constexpr_fn (t))
 	  && !type_dependent_expression_p (t)
 	  && !decl_maybe_constant_var_p (t)
 	  && (strict
@@ -9737,7 +9737,7 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
         STRIP_NOPS (x);
         if (is_this_parameter (x) && !is_capture_proxy (x))
 	  {
-	    if (!var_in_maybe_constexpr_fn (x))
+	    if (now || !var_in_maybe_constexpr_fn (x))
 	      {
 		if (flags & tf_error)
 		  constexpr_error (loc, fundef_p, "use of %<this%> in a "
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-fn8.C b/gcc/testsuite/g++.dg/cpp2a/concepts-fn8.C
new file mode 100644
index 00000000000..3f63a5b28d7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-fn8.C
@@ -0,0 +1,24 @@
+// PR c++/111703
+// { dg-do compile { target c++20 } }
+
+template<class T>
+constexpr bool always_true() { return true; }
+
+struct P {
+  P() = default;
+
+  template<class T>
+    requires (always_true<T>()) // { dg-bogus "used before its definition" }
+  constexpr P(const T&) { }
+
+  int n, m;
+};
+
+void (*f)(P);
+
+template<class T>
+constexpr bool g() {
+  P x;
+  f(x); // { dg-bogus "from here" }
+  return true;
+}
-- 
2.42.0.526.g3130c155df


             reply	other threads:[~2023-10-31 18:17 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-31 18:17 Patrick Palka [this message]
2023-11-01 15:07 ` Patrick Palka
2023-11-10 14:49   ` Patrick Palka
2023-11-14 22:57   ` Jason Merrill

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=20231031181726.3944801-1-ppalka@redhat.com \
    --to=ppalka@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).