Message ID | 1458682553-20882-1-git-send-email-patrick@parcs.ath.cx |
---|---|
State | New |
Headers | show |
On 03/22/2016 05:35 PM, Patrick Palka wrote:
> + if (cp_unevaluated_operand == 0
Why check this here?
Jason
On Tue, Mar 22, 2016 at 6:00 PM, Jason Merrill <jason@redhat.com> wrote: > On 03/22/2016 05:35 PM, Patrick Palka wrote: >> >> + if (cp_unevaluated_operand == 0 > > > Why check this here? Just so that the change doesn't affect the behavior of tsubst_decl() when cp_unevaluated_operand != 0. Presumably the existing code (10 lines below) handles that case just fine.
On Tue, Mar 22, 2016 at 6:12 PM, Patrick Palka <patrick@parcs.ath.cx> wrote: > On Tue, Mar 22, 2016 at 6:00 PM, Jason Merrill <jason@redhat.com> wrote: >> On 03/22/2016 05:35 PM, Patrick Palka wrote: >>> >>> + if (cp_unevaluated_operand == 0 >> >> >> Why check this here? > > Just so that the change doesn't affect the behavior of tsubst_decl() > when cp_unevaluated_operand != 0. Presumably the existing code (10 > lines below) handles that case just fine. Turns out that without the check we can trigger the cxx_dialect >= cxx14 assert because in c++11 mode we can reach the assert through get_defaulted_eh_spec() which increments cp_unevaluated_operand and then calls get_nsdmi (..., /*in_ctor=*/false) causing current_class_ref to get set to a PLACEHOLDER_EXPR. So for example g++.dg/cpp0x/nsdmi-template2.C regresses with an ICE. So it seems the cp_unevaluated_operand != 0 check is necessary as long as the assert stays. There are no regressions if both the cp_unevaluated_operand check and the assert are removed however.
On 03/22/2016 07:12 PM, Patrick Palka wrote: > On Tue, Mar 22, 2016 at 6:12 PM, Patrick Palka <patrick@parcs.ath.cx> wrote: >> On Tue, Mar 22, 2016 at 6:00 PM, Jason Merrill <jason@redhat.com> wrote: >>> On 03/22/2016 05:35 PM, Patrick Palka wrote: >>>> >>>> + if (cp_unevaluated_operand == 0 >>> >>> >>> Why check this here? >> >> Just so that the change doesn't affect the behavior of tsubst_decl() >> when cp_unevaluated_operand != 0. Presumably the existing code (10 >> lines below) handles that case just fine. > > Turns out that without the check we can trigger the cxx_dialect >= > cxx14 assert because in c++11 mode we can reach the assert through > get_defaulted_eh_spec() which increments cp_unevaluated_operand and > then calls get_nsdmi (..., /*in_ctor=*/false) causing > current_class_ref to get set to a PLACEHOLDER_EXPR. > > So for example g++.dg/cpp0x/nsdmi-template2.C regresses with an ICE. > So it seems the cp_unevaluated_operand != 0 check is necessary as long > as the assert stays. > > There are no regressions if both the cp_unevaluated_operand check and > the assert are removed however. I think that's my preference. Jason
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index ebfc45b..49ef9d3 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -13878,10 +13878,22 @@ tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl) if (r == NULL_TREE) { /* We get here for a use of 'this' in an NSDMI. */ - if (DECL_NAME (t) == this_identifier - && current_function_decl - && DECL_CONSTRUCTOR_P (current_function_decl)) - return current_class_ptr; + if (DECL_NAME (t) == this_identifier) + { + /* We're processing an NSDMI as part of a constructor call. */ + if (current_function_decl + && DECL_CONSTRUCTOR_P (current_function_decl)) + return current_class_ptr; + + /* Or as part of an aggregate initialization. */ + if (cp_unevaluated_operand == 0 + && current_class_ref + && TREE_CODE (current_class_ref) == PLACEHOLDER_EXPR) + { + gcc_assert (cxx_dialect >= cxx14); + return current_class_ptr; + } + } /* This can happen for a parameter name used later in a function declaration (such as in a late-specified return type). Just diff --git a/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr5.C b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr5.C new file mode 100644 index 0000000..fe377c3 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr5.C @@ -0,0 +1,24 @@ +// PR c++/70332 +// { dg-do run { target c++14 } } + +template <class T> +struct C +{ + T m; + T *n = &m; +}; + +C<int> c { }; + +int +main () +{ + *c.n = 5; + if (c.m != 5) + __builtin_abort (); + + C<int> d { 10 }; + *d.n = *d.n + 1; + if (d.m != 11) + __builtin_abort (); +}