| Message ID | 20240717160052.21678-1-polacek@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: wrong error initializing empty class [PR115900] | expand |
On 7/17/24 12:00 PM, Marek Polacek wrote: > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/14? OK. > -- >8 -- > In r14-409, we started handling empty bases first in cxx_fold_indirect_ref_1 > so that we don't need to recurse and waste time. > > This caused a bogus "modifying a const object" error. I'm appending my > analysis from the PR, but basically, cxx_fold_indirect_ref now returns > a different object than before, and we mark the wrong thing as const, > but since we're initializing an empty object, we should avoid setting > the object constness. > > ~~ > Pre-r14-409: we're evaluating the call to C::C(), which is in the body of > B::B(), which is the body of D::D(&d): > > C::C ((struct C *) this, NON_LVALUE_EXPR <0>) > > It's a ctor so we get here: > > 3118 /* Remember the object we are constructing or destructing. */ > 3119 tree new_obj = NULL_TREE; > 3120 if (DECL_CONSTRUCTOR_P (fun) || DECL_DESTRUCTOR_P (fun)) > 3121 { > 3122 /* In a cdtor, it should be the first `this' argument. > 3123 At this point it has already been evaluated in the call > 3124 to cxx_bind_parameters_in_call. */ > 3125 new_obj = TREE_VEC_ELT (new_call.bindings, 0); > > new_obj=(struct C *) &d.D.2656 > > 3126 new_obj = cxx_fold_indirect_ref (ctx, loc, DECL_CONTEXT (fun), new_obj); > > new_obj=d.D.2656.D.2597 > > We proceed to evaluate the call, then we get here: > > 3317 /* At this point, the object's constructor will have run, so > 3318 the object is no longer under construction, and its possible > 3319 'const' semantics now apply. Make a note of this fact by > 3320 marking the CONSTRUCTOR TREE_READONLY. */ > 3321 if (new_obj && DECL_CONSTRUCTOR_P (fun)) > 3322 cxx_set_object_constness (ctx, new_obj, /*readonly_p=*/true, > 3323 non_constant_p, overflow_p); > > new_obj is still d.D.2656.D.2597, its type is "C", cxx_set_object_constness > doesn't set anything as const. This is fine. > > After r14-409: on line 3125, new_obj is (struct C *) &d.D.2656 as before, > but we go to cxx_fold_indirect_ref_1: > > 5739 if (is_empty_class (type) > 5740 && CLASS_TYPE_P (optype) > 5741 && lookup_base (optype, type, ba_any, NULL, tf_none, off)) > 5742 { > 5743 if (empty_base) > 5744 *empty_base = true; > 5745 return op; > > type is C, which is an empty class; optype is "const D", and C is a base of D. > So we return the VAR_DECL 'd'. Then we get to cxx_set_object_constness with > object=d, which is const, so we mark the constructor READONLY. > > Then we're evaluating A::A() which has > > ((A*)this)->data = 0; > > we evaluate the LHS to d.D.2656.a, for which the initializer is > {.D.2656={.a={.data=}}} which is TREE_READONLY and 'd' is const, so we think > we're modifying a const object and fail the constexpr evaluation. > > PR c++/115900 > > gcc/cp/ChangeLog: > > * constexpr.cc (cxx_eval_call_expression): Set new_obj to NULL_TREE > if cxx_fold_indirect_ref set empty_base to true. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp2a/constexpr-init23.C: New test. > --- > gcc/cp/constexpr.cc | 14 ++++++++---- > gcc/testsuite/g++.dg/cpp2a/constexpr-init23.C | 22 +++++++++++++++++++ > 2 files changed, 32 insertions(+), 4 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-init23.C > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc > index f12b1dfc46d..abd3b04ea7f 100644 > --- a/gcc/cp/constexpr.cc > +++ b/gcc/cp/constexpr.cc > @@ -3123,10 +3123,16 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, > At this point it has already been evaluated in the call > to cxx_bind_parameters_in_call. */ > new_obj = TREE_VEC_ELT (new_call.bindings, 0); > - new_obj = cxx_fold_indirect_ref (ctx, loc, DECL_CONTEXT (fun), new_obj); > - > - if (ctx->call && ctx->call->fundef > - && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl)) > + bool empty_base = false; > + new_obj = cxx_fold_indirect_ref (ctx, loc, DECL_CONTEXT (fun), new_obj, > + &empty_base); > + /* If we're initializing an empty class, don't set constness, because > + cxx_fold_indirect_ref will return the wrong object to set constness > + of. */ > + if (empty_base) > + new_obj = NULL_TREE; > + else if (ctx->call && ctx->call->fundef > + && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl)) > { > tree cur_obj = TREE_VEC_ELT (ctx->call->bindings, 0); > STRIP_NOPS (cur_obj); > diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-init23.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-init23.C > new file mode 100644 > index 00000000000..466236d446d > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-init23.C > @@ -0,0 +1,22 @@ > +// PR c++/115900 > +// { dg-do compile { target c++20 } } > + > +struct A { > + char m; > + constexpr A() { m = 0; } > +}; > + > +struct C { > + constexpr C(){ }; > +}; > + > +struct B : C { > + A a; > + constexpr B() {} > +}; > + > +struct D : B { }; > + > +static constexpr A a; > +static constexpr B b; > +static constexpr D d; > > base-commit: e21fef7da92ef36af1e1b020ae5f35ef4f3c3fce
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc index f12b1dfc46d..abd3b04ea7f 100644 --- a/gcc/cp/constexpr.cc +++ b/gcc/cp/constexpr.cc @@ -3123,10 +3123,16 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, At this point it has already been evaluated in the call to cxx_bind_parameters_in_call. */ new_obj = TREE_VEC_ELT (new_call.bindings, 0); - new_obj = cxx_fold_indirect_ref (ctx, loc, DECL_CONTEXT (fun), new_obj); - - if (ctx->call && ctx->call->fundef - && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl)) + bool empty_base = false; + new_obj = cxx_fold_indirect_ref (ctx, loc, DECL_CONTEXT (fun), new_obj, + &empty_base); + /* If we're initializing an empty class, don't set constness, because + cxx_fold_indirect_ref will return the wrong object to set constness + of. */ + if (empty_base) + new_obj = NULL_TREE; + else if (ctx->call && ctx->call->fundef + && DECL_CONSTRUCTOR_P (ctx->call->fundef->decl)) { tree cur_obj = TREE_VEC_ELT (ctx->call->bindings, 0); STRIP_NOPS (cur_obj); diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-init23.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-init23.C new file mode 100644 index 00000000000..466236d446d --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-init23.C @@ -0,0 +1,22 @@ +// PR c++/115900 +// { dg-do compile { target c++20 } } + +struct A { + char m; + constexpr A() { m = 0; } +}; + +struct C { + constexpr C(){ }; +}; + +struct B : C { + A a; + constexpr B() {} +}; + +struct D : B { }; + +static constexpr A a; +static constexpr B b; +static constexpr D d;
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/14? -- >8 -- In r14-409, we started handling empty bases first in cxx_fold_indirect_ref_1 so that we don't need to recurse and waste time. This caused a bogus "modifying a const object" error. I'm appending my analysis from the PR, but basically, cxx_fold_indirect_ref now returns a different object than before, and we mark the wrong thing as const, but since we're initializing an empty object, we should avoid setting the object constness. ~~ Pre-r14-409: we're evaluating the call to C::C(), which is in the body of B::B(), which is the body of D::D(&d): C::C ((struct C *) this, NON_LVALUE_EXPR <0>) It's a ctor so we get here: 3118 /* Remember the object we are constructing or destructing. */ 3119 tree new_obj = NULL_TREE; 3120 if (DECL_CONSTRUCTOR_P (fun) || DECL_DESTRUCTOR_P (fun)) 3121 { 3122 /* In a cdtor, it should be the first `this' argument. 3123 At this point it has already been evaluated in the call 3124 to cxx_bind_parameters_in_call. */ 3125 new_obj = TREE_VEC_ELT (new_call.bindings, 0); new_obj=(struct C *) &d.D.2656 3126 new_obj = cxx_fold_indirect_ref (ctx, loc, DECL_CONTEXT (fun), new_obj); new_obj=d.D.2656.D.2597 We proceed to evaluate the call, then we get here: 3317 /* At this point, the object's constructor will have run, so 3318 the object is no longer under construction, and its possible 3319 'const' semantics now apply. Make a note of this fact by 3320 marking the CONSTRUCTOR TREE_READONLY. */ 3321 if (new_obj && DECL_CONSTRUCTOR_P (fun)) 3322 cxx_set_object_constness (ctx, new_obj, /*readonly_p=*/true, 3323 non_constant_p, overflow_p); new_obj is still d.D.2656.D.2597, its type is "C", cxx_set_object_constness doesn't set anything as const. This is fine. After r14-409: on line 3125, new_obj is (struct C *) &d.D.2656 as before, but we go to cxx_fold_indirect_ref_1: 5739 if (is_empty_class (type) 5740 && CLASS_TYPE_P (optype) 5741 && lookup_base (optype, type, ba_any, NULL, tf_none, off)) 5742 { 5743 if (empty_base) 5744 *empty_base = true; 5745 return op; type is C, which is an empty class; optype is "const D", and C is a base of D. So we return the VAR_DECL 'd'. Then we get to cxx_set_object_constness with object=d, which is const, so we mark the constructor READONLY. Then we're evaluating A::A() which has ((A*)this)->data = 0; we evaluate the LHS to d.D.2656.a, for which the initializer is {.D.2656={.a={.data=}}} which is TREE_READONLY and 'd' is const, so we think we're modifying a const object and fail the constexpr evaluation. PR c++/115900 gcc/cp/ChangeLog: * constexpr.cc (cxx_eval_call_expression): Set new_obj to NULL_TREE if cxx_fold_indirect_ref set empty_base to true. gcc/testsuite/ChangeLog: * g++.dg/cpp2a/constexpr-init23.C: New test. --- gcc/cp/constexpr.cc | 14 ++++++++---- gcc/testsuite/g++.dg/cpp2a/constexpr-init23.C | 22 +++++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-init23.C base-commit: e21fef7da92ef36af1e1b020ae5f35ef4f3c3fce