| Message ID | 66ecb98f.050a0220.2efcb2.522d@mx.google.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] c++: Don't strip USING_DECLs when updating local bindings [PR116748] | expand |
On 9/19/24 7:53 PM, Nathaniel Shead wrote: > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? OK. > Alternatively I could solve this the other way around (have a new > 'old_target = strip_using_decl (old)' and replace all usages of 'old' > except the usages in this patch); this is more churn but probably better > matches how other functions are structured. > > -- >8 -- > > Currently update_binding strips USING_DECLs too eagerly, leading to ICEs > in pop_local_decl as it can't find the decl it's popping in the binding > list. Let's rather try to keep the original USING_DECL around. > > This also means that using59.C can point to the location of the > using-decl rather than the underlying object directly; this is in the > direction required to fix PR c++/106851 (though more work is needed to > emit properly helpful diagnostics here). > > PR c++/116748 > > gcc/cp/ChangeLog: > > * name-lookup.cc (update_binding): Maintain USING_DECLs in the > binding slots. > > gcc/testsuite/ChangeLog: > > * g++.dg/lookup/using59.C: Update location. > * g++.dg/lookup/using69.C: New test. > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > --- > gcc/cp/name-lookup.cc | 12 +++++++----- > gcc/testsuite/g++.dg/lookup/using59.C | 4 ++-- > gcc/testsuite/g++.dg/lookup/using69.C | 10 ++++++++++ > 3 files changed, 19 insertions(+), 7 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/lookup/using69.C > > diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc > index c7a693e02d5..94b031e6be2 100644 > --- a/gcc/cp/name-lookup.cc > +++ b/gcc/cp/name-lookup.cc > @@ -3005,6 +3005,8 @@ update_binding (cp_binding_level *level, cxx_binding *binding, tree *slot, > > if (old == error_mark_node) > old = NULL_TREE; > + > + tree old_bval = old; > old = strip_using_decl (old); > > if (DECL_IMPLICIT_TYPEDEF_P (decl)) > @@ -3021,7 +3023,7 @@ update_binding (cp_binding_level *level, cxx_binding *binding, tree *slot, > gcc_checking_assert (!to_type); > hide_type = hiding; > to_type = decl; > - to_val = old; > + to_val = old_bval; > } > else > hide_value = hiding; > @@ -3034,7 +3036,7 @@ update_binding (cp_binding_level *level, cxx_binding *binding, tree *slot, > /* OLD is an implicit typedef. Move it to to_type. */ > gcc_checking_assert (!to_type); > > - to_type = old; > + to_type = old_bval; > hide_type = hide_value; > old = NULL_TREE; > hide_value = false; > @@ -3093,7 +3095,7 @@ update_binding (cp_binding_level *level, cxx_binding *binding, tree *slot, > { > if (same_type_p (TREE_TYPE (old), TREE_TYPE (decl))) > /* Two type decls to the same type. Do nothing. */ > - return old; > + return old_bval; > else > goto conflict; > } > @@ -3106,7 +3108,7 @@ update_binding (cp_binding_level *level, cxx_binding *binding, tree *slot, > > /* The new one must be an alias at this point. */ > gcc_assert (DECL_NAMESPACE_ALIAS (decl)); > - return old; > + return old_bval; > } > else if (TREE_CODE (old) == VAR_DECL) > { > @@ -3121,7 +3123,7 @@ update_binding (cp_binding_level *level, cxx_binding *binding, tree *slot, > else > { > conflict: > - diagnose_name_conflict (decl, old); > + diagnose_name_conflict (decl, old_bval); > to_val = NULL_TREE; > } > } > diff --git a/gcc/testsuite/g++.dg/lookup/using59.C b/gcc/testsuite/g++.dg/lookup/using59.C > index 3c3a73c28d5..b7ec325d234 100644 > --- a/gcc/testsuite/g++.dg/lookup/using59.C > +++ b/gcc/testsuite/g++.dg/lookup/using59.C > @@ -1,10 +1,10 @@ > > namespace Y > { > - extern int I; // { dg-message "previous declaration" } > + extern int I; > } > > -using Y::I; > +using Y::I; // { dg-message "previous declaration" } > extern int I; // { dg-error "conflicts with a previous" } > > extern int J; > diff --git a/gcc/testsuite/g++.dg/lookup/using69.C b/gcc/testsuite/g++.dg/lookup/using69.C > new file mode 100644 > index 00000000000..7d52b73b9ce > --- /dev/null > +++ b/gcc/testsuite/g++.dg/lookup/using69.C > @@ -0,0 +1,10 @@ > +// PR c++/116748 > + > +namespace ns { > + struct empty; > +} > + > +void foo() { > + using ns::empty; > + int empty; > +}
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc index c7a693e02d5..94b031e6be2 100644 --- a/gcc/cp/name-lookup.cc +++ b/gcc/cp/name-lookup.cc @@ -3005,6 +3005,8 @@ update_binding (cp_binding_level *level, cxx_binding *binding, tree *slot, if (old == error_mark_node) old = NULL_TREE; + + tree old_bval = old; old = strip_using_decl (old); if (DECL_IMPLICIT_TYPEDEF_P (decl)) @@ -3021,7 +3023,7 @@ update_binding (cp_binding_level *level, cxx_binding *binding, tree *slot, gcc_checking_assert (!to_type); hide_type = hiding; to_type = decl; - to_val = old; + to_val = old_bval; } else hide_value = hiding; @@ -3034,7 +3036,7 @@ update_binding (cp_binding_level *level, cxx_binding *binding, tree *slot, /* OLD is an implicit typedef. Move it to to_type. */ gcc_checking_assert (!to_type); - to_type = old; + to_type = old_bval; hide_type = hide_value; old = NULL_TREE; hide_value = false; @@ -3093,7 +3095,7 @@ update_binding (cp_binding_level *level, cxx_binding *binding, tree *slot, { if (same_type_p (TREE_TYPE (old), TREE_TYPE (decl))) /* Two type decls to the same type. Do nothing. */ - return old; + return old_bval; else goto conflict; } @@ -3106,7 +3108,7 @@ update_binding (cp_binding_level *level, cxx_binding *binding, tree *slot, /* The new one must be an alias at this point. */ gcc_assert (DECL_NAMESPACE_ALIAS (decl)); - return old; + return old_bval; } else if (TREE_CODE (old) == VAR_DECL) { @@ -3121,7 +3123,7 @@ update_binding (cp_binding_level *level, cxx_binding *binding, tree *slot, else { conflict: - diagnose_name_conflict (decl, old); + diagnose_name_conflict (decl, old_bval); to_val = NULL_TREE; } } diff --git a/gcc/testsuite/g++.dg/lookup/using59.C b/gcc/testsuite/g++.dg/lookup/using59.C index 3c3a73c28d5..b7ec325d234 100644 --- a/gcc/testsuite/g++.dg/lookup/using59.C +++ b/gcc/testsuite/g++.dg/lookup/using59.C @@ -1,10 +1,10 @@ namespace Y { - extern int I; // { dg-message "previous declaration" } + extern int I; } -using Y::I; +using Y::I; // { dg-message "previous declaration" } extern int I; // { dg-error "conflicts with a previous" } extern int J; diff --git a/gcc/testsuite/g++.dg/lookup/using69.C b/gcc/testsuite/g++.dg/lookup/using69.C new file mode 100644 index 00000000000..7d52b73b9ce --- /dev/null +++ b/gcc/testsuite/g++.dg/lookup/using69.C @@ -0,0 +1,10 @@ +// PR c++/116748 + +namespace ns { + struct empty; +} + +void foo() { + using ns::empty; + int empty; +}
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? Alternatively I could solve this the other way around (have a new 'old_target = strip_using_decl (old)' and replace all usages of 'old' except the usages in this patch); this is more churn but probably better matches how other functions are structured. -- >8 -- Currently update_binding strips USING_DECLs too eagerly, leading to ICEs in pop_local_decl as it can't find the decl it's popping in the binding list. Let's rather try to keep the original USING_DECL around. This also means that using59.C can point to the location of the using-decl rather than the underlying object directly; this is in the direction required to fix PR c++/106851 (though more work is needed to emit properly helpful diagnostics here). PR c++/116748 gcc/cp/ChangeLog: * name-lookup.cc (update_binding): Maintain USING_DECLs in the binding slots. gcc/testsuite/ChangeLog: * g++.dg/lookup/using59.C: Update location. * g++.dg/lookup/using69.C: New test. Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> --- gcc/cp/name-lookup.cc | 12 +++++++----- gcc/testsuite/g++.dg/lookup/using59.C | 4 ++-- gcc/testsuite/g++.dg/lookup/using69.C | 10 ++++++++++ 3 files changed, 19 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/g++.dg/lookup/using69.C