[1/2] c++: Don't strip USING_DECLs when updating local bindings [PR116748]

  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
  

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;
> +}