[PATCH] c++: Fix constexpr evaluation of pre-increment when !lval [PR99287]

[Patrick Palka]

Here, during cxx_eval_increment_expression (with lval=false) of
++__first where __first is &"mystr"[0], we correctly update __first
to &"mystr"[1] but we end up returning &"mystr"[0] + 1 instead of
&"mystr"[1].  This unreduced return value inhibits other pointer
arithmetic folding during later constexpr evaluation, which ultimately
causes the constexpr evaluation to fail.

It turns out the simplification of &"mystr"[0] + 1 to &"mystr"[1]
is performed by cxx_fold_pointer_plus_expression, not by fold_build2.
So we perform this simplification during constexpr evaluation of
the temporary MODIFY_EXPR (assigning to __first the simplified value),
but then we return 'mod' which has only been folded via fold_build2 and
hasn't gone through cxx_fold_pointer_plus_expression.

This patch fixes this by updating 'mod' to the (rvalue) result of the
MODIFY_EXPR evaluation, so that we capture any additional folding of
'mod'.  We now need to be wary of the evaluation failing and returning
e.g. the MODIFY_EXPR or NULL_TREE; it seems checking *non_constant_p
should cover our bases here and is generally prudent.

(Finally, since returning 'mod' instead of 'op' when !lval seems to be
more than just an optimization, i.e. callers seems to expect this
behavior, this patch additionally clarifies the nearby comment to that
effect.)

Boostrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk or perhaps GCC 12?

gcc/cp/ChangeLog:

	PR c++/99287
	* constexpr.c (cxx_eval_increment_expression): Pass lval=false
	when evaluating the MODIFY_EXPR, and update 'mod' with the
	result of this evaluation.  Check *non_constant_p afterwards.
	Clarify nearby comment.

gcc/testsuite/ChangeLog:

	PR c++/99287
	* g++.dg/cpp2a/constexpr-99287.C: New test.

Co-authored-by: Jakub Jelinek <jakub@redhat.com>
---
 gcc/cp/constexpr.c                           | 16 ++---
 gcc/testsuite/g++.dg/cpp2a/constexpr-99287.C | 61 ++++++++++++++++++++
 2 files changed, 67 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-99287.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index cd0a68e9fd6..49df79837ca 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -5582,20 +5582,16 @@ cxx_eval_increment_expression (const constexpr_ctx *ctx, tree t,
   /* Storing the modified value.  */
   tree store = build2_loc (cp_expr_loc_or_loc (t, input_location),
 			   MODIFY_EXPR, type, op, mod);
-  cxx_eval_constant_expression (ctx, store,
-				true, non_constant_p, overflow_p);
+  mod = cxx_eval_constant_expression (ctx, store, false,
+				      non_constant_p, overflow_p);
   ggc_free (store);
+  if (*non_constant_p)
+    return t;
 
   /* And the value of the expression.  */
   if (code == PREINCREMENT_EXPR || code == PREDECREMENT_EXPR)
-    {
-      /* Prefix ops are lvalues.  */
-      if (lval)
-	return op;
-      else
-	/* But we optimize when the caller wants an rvalue.  */
-	return mod;
-    }
+    /* Prefix ops are lvalues, but the caller might want an rvalue.  */
+    return lval ? op : mod;
   else
     /* Postfix ops are rvalues.  */
     return val;
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-99287.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-99287.C
new file mode 100644
index 00000000000..c9c2ac2f7c2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-99287.C
@@ -0,0 +1,61 @@
+// PR c++/99287
+// { dg-do compile { target c++20 } }
+
+namespace std {
+struct source_location {
+  static consteval source_location
+  current(const void *__p = __builtin_source_location()) {
+    source_location __ret;
+    __ret._M_impl = static_cast<const __impl *>(__p);
+    return __ret;
+  }
+  constexpr const char *function_name() {
+    return _M_impl ? _M_impl->_M_function_name : "";
+  }
+  struct __impl {
+    const char *_M_file_name;
+    const char *_M_function_name;
+    unsigned _M_line;
+    unsigned _M_column;
+  } const *_M_impl;
+};
+struct char_traits {
+  static constexpr long length(const char *__s) {
+    return __builtin_strlen(__s);
+  }
+};
+template <typename _CharT, typename _Traits = char_traits>
+class basic_string_view {
+public:
+  using traits_type = _Traits;
+  using size_type = unsigned long;
+  constexpr basic_string_view(const _CharT *__str)
+      : _M_len{traits_type::length(__str)}, _M_str{__str} {}
+  constexpr size_type find(const _CharT *, size_type, size_type) const noexcept;
+  constexpr size_type find(_CharT *__str) {
+    long __trans_tmp_1 = traits_type::length(__str);
+    return find(__str, 0, __trans_tmp_1);
+  }
+  long _M_len;
+  const _CharT *_M_str;
+};
+using string_view = basic_string_view<const char>;
+template <typename _CharT, typename _Traits>
+constexpr unsigned long
+basic_string_view<_CharT, _Traits>::find(const _CharT *__str, size_type,
+                                         size_type __n) const noexcept {
+  int __trans_tmp_2;
+  const _CharT *__first = _M_str;
+  size_type __len = _M_len;
+  while (__len >= __n) {
+    __trans_tmp_2 = __builtin_memcmp(__first, __str, __n);
+    if (__trans_tmp_2 == 0)
+      return __first - _M_str;
+    __len = _M_str - ++__first;
+  }
+}
+} // namespace std
+template <typename> consteval auto f() {
+  return std::string_view{std::source_location::current().function_name()};
+}
+int main() { constexpr auto s = f<int>().find("int"); }
-- 
2.31.0.rc0.75.gec125d1bc1