On Fri, 5 Mar 2021, Jason Merrill wrote:
> On 3/5/21 1:05 PM, Patrick Palka wrote:
> > 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 <ja...@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,
>
> How about passing lval down here and returning mod either way?
Sounds good, like this? Testing in progress
-- >8 --
Subject: [PATCH] c++: Fix constexpr evaluation of pre-increment when !lval
[PR99287]
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 (during which we assign 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 also updating 'mod' with the result of the
MODIFY_EXPR evaluation appropriately, so that we capture any additional
folding of the expression when !lval. We now need to be wary of this
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.
gcc/cp/ChangeLog:
PR c++/99287
* constexpr.c (cxx_eval_increment_expression): Pass lval when
evaluating the MODIFY_EXPR, and update 'mod' with the result of
this evaluation. Check *non_constant_p afterwards. For prefix
ops, just return 'mod'.
gcc/testsuite/ChangeLog:
PR c++/99287
* g++.dg/cpp2a/constexpr-99287.C: New test.
Co-authored-by: Jakub Jelinek <ja...@redhat.com>
---
gcc/cp/constexpr.c | 17 +++---
gcc/testsuite/g++.dg/cpp2a/constexpr-99287.C | 61 ++++++++++++++++++++
2 files changed, 68 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 7d96d577d84..d7150b25b19 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -5582,20 +5582,17 @@ 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, lval,
+ 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.
+ This has already been taken into account by the store above. */
+ return 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
