On Fri, Mar 07, 2025 at 12:00:00PM -0500, Jason Merrill wrote:
> On 3/6/25 5:13 PM, Marek Polacek wrote:
> > On Wed, Mar 05, 2025 at 05:28:49PM -0500, Jason Merrill wrote:
> > > On 3/5/25 4:00 PM, Marek Polacek wrote:
> > > > On Wed, Mar 05, 2025 at 03:31:59PM -0500, Jason Merrill wrote:
> > > > > On 3/5/25 12:09 PM, Marek Polacek wrote:
> > > > > > On Tue, Mar 04, 2025 at 05:34:10PM -0500, Jason Merrill wrote:
> > > > > > > On 2/11/25 6:24 PM, Marek Polacek wrote:
> > > > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > > > > > > 
> > > > > > > > -- >8 --
> > > > > > > > Here we ICE since r11-7740 because we no longer say that 
> > > > > > > > (long)&a
> > > > > > > > (where a is a global var) is non_constant_p.  So VERIFY_CONSTANT
> > > > > > > > does not return and we crash on tree_to_uhwi.  We should check
> > > > > > > > tree_fits_uhwi_p before calling tree_to_uhwi.
> > > > > > > > 
> > > > > > > >         PR c++/118775
> > > > > > > > 
> > > > > > > > gcc/cp/ChangeLog:
> > > > > > > > 
> > > > > > > >         * constexpr.cc (cxx_eval_call_expression): Check 
> > > > > > > > tree_fits_uhwi_p.
> > > > > > > > 
> > > > > > > > gcc/testsuite/ChangeLog:
> > > > > > > > 
> > > > > > > >         * g++.dg/cpp2a/constexpr-new24.C: New test.
> > > > > > > >         * g++.dg/cpp2a/constexpr-new25.C: New test.
> > > > > > > > ---
> > > > > > > >      gcc/cp/constexpr.cc                          |  7 +++++
> > > > > > > >      gcc/testsuite/g++.dg/cpp2a/constexpr-new24.C | 25 
> > > > > > > > ++++++++++++++++++
> > > > > > > >      gcc/testsuite/g++.dg/cpp2a/constexpr-new25.C | 27 
> > > > > > > > ++++++++++++++++++++
> > > > > > > >      3 files changed, 59 insertions(+)
> > > > > > > >      create mode 100644 
> > > > > > > > gcc/testsuite/g++.dg/cpp2a/constexpr-new24.C
> > > > > > > >      create mode 100644 
> > > > > > > > gcc/testsuite/g++.dg/cpp2a/constexpr-new25.C
> > > > > > > > 
> > > > > > > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> > > > > > > > index f142dd32bc8..f8f9a9df1a2 100644
> > > > > > > > --- a/gcc/cp/constexpr.cc
> > > > > > > > +++ b/gcc/cp/constexpr.cc
> > > > > > > > @@ -2909,6 +2909,13 @@ cxx_eval_call_expression (const 
> > > > > > > > constexpr_ctx *ctx, tree t,
> > > > > > > >           gcc_assert (arg0);
> > > > > > > >           if (new_op_p)
> > > > > > > >             {
> > > > > > > > +             if (!tree_fits_uhwi_p (arg0))
> > > > > > > > +               {
> > > > > > > > +                 if (!ctx->quiet)
> > > > > > > > +                   error_at (loc, "cannot allocate array: size 
> > > > > > > > too large");
> > > > > > > 
> > > > > > > "too large" seems misleading in this case, where it just isn't a
> > > > > > > compile-time constant.
> > > > > > 
> > > > > > Fair, how about "size not constant"?
> > > > > > > Why didn't the VERIFY_CONSTANT just above already reject this?
> > > > > > 
> > > > > > This is about *non_constant_p.  Since r11-7740 
> > > > > > cxx_eval_constant_expression
> > > > > > returns early less often:
> > > > > > 
> > > > > > @@ -6656,7 +6656,8 @@ cxx_eval_constant_expression (const 
> > > > > > constexpr_ctx *ctx, tree t,
> > > > > > 
> > > > > >        if (TREE_CODE (t) == CONVERT_EXPR
> > > > > >            && ARITHMETIC_TYPE_P (type)
> > > > > > -       && INDIRECT_TYPE_P (TREE_TYPE (op)))
> > > > > > +       && INDIRECT_TYPE_P (TREE_TYPE (op))
> > > > > > +       && ctx->manifestly_const_eval)
> > > > > 
> > > > > Aha.  I think this should check ctx->strict instead of
> > > > > ctx->manifestly_const_eval.
> > > > 
> > > > In the r11-7740 patch Jakub mentioned he had tried that, but it 
> > > > regressed
> > > > some tests.  I've tried to see if that is still the case and it is; at
> > > > least
> > > > g++.dg/cpp1y/constexpr-shift1.C
> > > > g++.dg/cpp1y/constexpr-82304.C
> > > > g++.dg/cpp0x/constexpr-ex1.C
> > > > FAIL with ctx->strict.
> > > 
> > > This seems to be because maybe_constant_init_1 wrongly chooses non-strict
> > > mode for static constexpr variables.  After correcting that, only
> > > constexpr-new3.C fails, which seems to be a latent bug that we can 
> > > otherwise
> > > see by moving v7 into a function.
> > 
> > Ah yeah,
> > 
> > @@ -9649,7 +9651,9 @@ maybe_constant_init_1 (tree t, tree decl, bool 
> > allow_non_constant,
> >       static or thread storage duration even if it isn't required, but we
> >       shouldn't bend the rules the same way for automatic variables.  */
> >         bool is_static = (decl && DECL_P (decl)
> > -           && (TREE_STATIC (decl) || DECL_EXTERNAL (decl)));
> > +           && ((TREE_STATIC (decl)
> > +                && !DECL_DECLARED_CONSTEXPR_P (decl))
> > +               || DECL_EXTERNAL (decl)));
> >         if (is_static)
> >      manifestly_const_eval = true;
> > does seem to work.  I don't know how I would fix constexpr-new3.C though.
> 
> I'll follow up on this.
 
Thanks.  I created <https://gcc.gnu.org/PR119162>.

> > > > Also, while using ctx->strict fixes constexpr-new25.C, it doesn't fix
> > > > the constexpr-new24.C crash.
> > > 
> > > Why?
> > 
> > Took me a sec but it's because there's no CONVERT_EXPR, only NOP_EXPR.
> > In that test we start off with:
> > 
> >    1 * (sizetype) (long int) &a
> > 
> > where (long int) &a is a CONVERT_EXPR, but after the
> > 
> >         size = cp_fully_fold (size);
> > 
> > in build_new_1 we get:
> > 
> >    (sizetype) &a
> > 
> > and the CONVERT_EXPR is gone.  Expanding the "conversion from pointer type"
> > check to NOP_EXPR breaks a lot of other tests.  Sigh.
> 
> Hmm, we should probably remove that cp_fully_fold or change it to
> maybe_constant_value/fold_simple/fold_to_constant.
> 
> But let's go ahead with your v2 patch, with a FIXME that the VERIFY_CONSTANT
> should have already caught it.

Sounds good, thanks.  Here's what I'm pushing:

-- >8 --
Here we ICE since r11-7740 because we no longer say that (long)&a
(where a is a global var) is non_constant_p.  So VERIFY_CONSTANT
does not return and we crash on tree_to_uhwi.  We should check
tree_fits_uhwi_p before calling tree_to_uhwi.

        PR c++/118775

gcc/cp/ChangeLog:

        * constexpr.cc (cxx_eval_call_expression): Check tree_fits_uhwi_p.

gcc/testsuite/ChangeLog:

        * g++.dg/cpp2a/constexpr-new24.C: New test.
        * g++.dg/cpp2a/constexpr-new25.C: New test.

Reviewed-by: Jason Merrill <ja...@redhat.com>
---
 gcc/cp/constexpr.cc                          | 11 ++++++++
 gcc/testsuite/g++.dg/cpp2a/constexpr-new24.C | 25 ++++++++++++++++++
 gcc/testsuite/g++.dg/cpp2a/constexpr-new25.C | 27 ++++++++++++++++++++
 3 files changed, 63 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-new24.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-new25.C

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 88b070d8a8e..381e5e294bd 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -2928,6 +2928,17 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree 
t,
          gcc_assert (arg0);
          if (new_op_p)
            {
+             /* FIXME: We should not get here; the VERIFY_CONSTANT above
+                should have already caught it.  But currently a conversion
+                from pointer type to arithmetic type is only considered
+                non-constant for CONVERT_EXPRs, not NOP_EXPRs.  */
+             if (!tree_fits_uhwi_p (arg0))
+               {
+                 if (!ctx->quiet)
+                   error_at (loc, "cannot allocate array: size not constant");
+                 *non_constant_p = true;
+                 return t;
+               }
              tree type = build_array_type_nelts (char_type_node,
                                                  tree_to_uhwi (arg0));
              tree var = build_decl (loc, VAR_DECL,
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-new24.C 
b/gcc/testsuite/g++.dg/cpp2a/constexpr-new24.C
new file mode 100644
index 00000000000..ee62f18922c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-new24.C
@@ -0,0 +1,25 @@
+// PR c++/118775
+// { dg-do compile { target c++20 } }
+
+int a;
+
+constexpr char *
+f1 ()
+{
+  constexpr auto p = new char[(long int) &a]; // { dg-error "size not 
constant" }
+  return p;
+}
+
+constexpr char *
+f2 ()
+{
+  auto p = new char[(long int) &a];  // { dg-error "size not constant" }
+  return p;
+}
+
+void
+g ()
+{
+  auto r1 = f2 ();
+  constexpr auto r2 = f2 (); // { dg-message "in .constexpr. expansion" }
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-new25.C 
b/gcc/testsuite/g++.dg/cpp2a/constexpr-new25.C
new file mode 100644
index 00000000000..91c0318abd8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-new25.C
@@ -0,0 +1,27 @@
+// PR c++/118775
+// { dg-do compile { target c++20 } }
+
+namespace std {
+struct __uniq_ptr_impl {
+  constexpr __uniq_ptr_impl(char *) {}
+};
+template <typename> struct unique_ptr {
+  __uniq_ptr_impl _M_t;
+  constexpr ~unique_ptr() {}
+};
+template <typename> struct _MakeUniq;
+template <typename _Tp> struct _MakeUniq<_Tp[]> {
+  typedef unique_ptr<_Tp[]> __array;
+};
+template <typename _Tp> using __unique_ptr_array_t = _MakeUniq<_Tp>::__array;
+constexpr __unique_ptr_array_t<char[]> make_unique(long __num) {
+  return unique_ptr<char[]>(new char[__num]);
+}
+} // namespace std
+int a;
+int
+main ()
+{
+  std::unique_ptr p = std::make_unique((long)&a);
+  constexpr std::unique_ptr p2 = std::make_unique((long)&a); // { dg-error 
"conversion" }
+}

base-commit: c781da2c10641a651019f8fe77f57ccdbc49f024
-- 
2.48.1

Reply via email to