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