diff mbox series

[pushed] c++: using lookup within class defn [PR104476]

Message ID 20220318180746.1960276-1-jason@redhat.com
State New
Headers show
Series [pushed] c++: using lookup within class defn [PR104476] | expand

Commit Message

Jason Merrill March 18, 2022, 6:07 p.m. UTC
The problem in both PR92918 and PR104476 is overloading of base member
functions brought in by 'using' with direct member functions during parsing
of the class body.  To this point they've had a troublesome coexistence
which was resolved by set_class_bindings when the class is complete, but we
also need to handle lookup within the class body, such as in a trailing
return type.

The problem was that push_class_level_binding would either clobber the
using-decl with the direct members or vice-versa.  In older versions of GCC
we only pushed dependent usings, and preferring the dependent using made
sense, as it expresses a type-dependent overload set that we can't do
anything useful with.  But when we started keeping non-dependent usings
around, push_class_level_binding in particular wasn't adjusted accordingly.

This patch makes that adjustment, and pushes the functions imported by a
non-dependent using immediately from finish_member_declaration.  This made
diagnosing redundant using-decls a bit awkward, since we no longer push the
using-decl itself; I handle that by noticing when we try to add the same
function again and searching TYPE_FIELDS for the previous using-decl.

Tested x86_64-pc-linux-gnu, applying to trunk.

	PR c++/92918
	PR c++/104476

gcc/cp/ChangeLog:

	* class.cc (add_method): Avoid adding the same used function twice.
	(handle_using_decl): Don't add_method.
	(finish_struct): Don't using op= if we have one already.
	(maybe_push_used_methods): New.
	* semantics.cc (finish_member_declaration): Call it.
	* name-lookup.cc (diagnose_name_conflict): No longer static.
	(push_class_level_binding): Revert 92918 patch, limit
	to dependent using.
	* cp-tree.h: Adjust.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/pr85070.C: Remove expected error.
	* g++.dg/lookup/using66a.C: New test.
	* g++.dg/lookup/using67.C: New test.
---
 gcc/cp/cp-tree.h                       |   2 +
 gcc/cp/class.cc                        | 136 +++++++++++++++----------
 gcc/cp/name-lookup.cc                  |  16 ++-
 gcc/cp/semantics.cc                    |   5 +-
 gcc/testsuite/g++.dg/cpp0x/pr85070.C   |   4 +-
 gcc/testsuite/g++.dg/lookup/using66a.C |  22 ++++
 gcc/testsuite/g++.dg/lookup/using67.C  |  20 ++++
 7 files changed, 142 insertions(+), 63 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/lookup/using66a.C
 create mode 100644 gcc/testsuite/g++.dg/lookup/using67.C


base-commit: 16fe6e8c9787a3be47bee3dbf094d22a5691c190
diff mbox series

Patch

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index a4bc48a4a20..1bd7bc6fca2 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6677,6 +6677,7 @@  extern tree build_vfn_ref			(tree, tree);
 extern tree get_vtable_decl			(tree, int);
 extern bool add_method				(tree, tree, bool);
 extern tree declared_access			(tree);
+extern bool maybe_push_used_methods		(tree);
 extern tree currently_open_class		(tree);
 extern tree currently_open_derived_class	(tree);
 extern tree outermost_open_class		(void);
@@ -8241,6 +8242,7 @@  extern tree fold_builtin_source_location	(location_t);
 
 /* in name-lookup.cc */
 extern tree strip_using_decl                    (tree);
+extern void diagnose_name_conflict		(tree, tree);
 
 /* Tell the binding oracle what kind of binding we are looking for.  */
 
diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
index 696155714e8..40e17140db5 100644
--- a/gcc/cp/class.cc
+++ b/gcc/cp/class.cc
@@ -1049,7 +1049,12 @@  add_method (tree type, tree method, bool via_using)
       if (via_using && iter.using_p ()
 	  /* Except handle inherited constructors specially.  */
 	  && ! DECL_CONSTRUCTOR_P (fn))
-	continue;
+	{
+	  if (fn == method)
+	    /* Don't add the same one twice.  */
+	    return false;
+	  continue;
+	}
 
       /* [over.load] Member function declarations with the
 	 same name and the same parameter types cannot be
@@ -1212,10 +1217,11 @@  add_method (tree type, tree method, bool via_using)
       if (via_using)
 	/* Defer to the local function.  */
 	return false;
-      else if (flag_new_inheriting_ctors
-	       && DECL_INHERITED_CTOR (fn))
+      else if (iter.using_p ()
+	       ||  (flag_new_inheriting_ctors
+		    && DECL_INHERITED_CTOR (fn)))
 	{
-	  /* Remove the inherited constructor.  */
+	  /* Remove the inherited function.  */
 	  current_fns = iter.remove_node (current_fns);
 	  continue;
 	}
@@ -1299,21 +1305,65 @@  declared_access (tree decl)
 	  : access_public_node);
 }
 
+/* If DECL is a non-dependent using of non-ctor function members, push them
+   and return true, otherwise return false.  Called from
+   finish_member_declaration.  */
+
+bool
+maybe_push_used_methods (tree decl)
+{
+  if (TREE_CODE (decl) != USING_DECL)
+    return false;
+  tree used = strip_using_decl (decl);
+  if (!used || !is_overloaded_fn (used))
+    return false;
+
+  /* Add the functions to CLASSTYPE_MEMBER_VEC so that overload resolution
+     works within the class body.  */
+  for (tree f : ovl_range (used))
+    {
+      if (DECL_CONSTRUCTOR_P (f))
+	/* Inheriting constructors are handled separately.  */
+	return false;
+
+      bool added = add_method (current_class_type, f, true);
+
+      if (added)
+	alter_access (current_class_type, f, current_access_specifier);
+
+      /* If add_method returns false because f was already declared, look
+	 for a duplicate using-declaration.  */
+      else
+	for (tree d = TYPE_FIELDS (current_class_type); d; d = DECL_CHAIN (d))
+	  if (TREE_CODE (d) == USING_DECL
+	      && DECL_NAME (d) == DECL_NAME (decl)
+	      && same_type_p (USING_DECL_SCOPE (d), USING_DECL_SCOPE (decl)))
+	    {
+	      diagnose_name_conflict (decl, d);
+	      break;
+	    }
+    }
+  return true;
+}
+
 /* Process the USING_DECL, which is a member of T.  */
 
 static void
 handle_using_decl (tree using_decl, tree t)
 {
   tree decl = USING_DECL_DECLS (using_decl);
-  tree name = DECL_NAME (using_decl);
-  tree access = declared_access (using_decl);
-  tree flist = NULL_TREE;
-  tree old_value;
 
   gcc_assert (!processing_template_decl && decl);
 
-  old_value = lookup_member (t, name, /*protect=*/0, /*want_type=*/false,
-			     tf_warning_or_error);
+  cp_emit_debug_info_for_using (decl, t);
+
+  if (is_overloaded_fn (decl))
+    /* Handled in maybe_push_used_methods.  */
+    return;
+
+  tree name = DECL_NAME (using_decl);
+  tree old_value = lookup_member (t, name, /*protect=*/0, /*want_type=*/false,
+				  tf_warning_or_error);
   if (old_value)
     {
       old_value = OVL_FIRST (old_value);
@@ -1324,27 +1374,16 @@  handle_using_decl (tree using_decl, tree t)
 	old_value = NULL_TREE;
     }
 
-  cp_emit_debug_info_for_using (decl, t);
-
-  if (is_overloaded_fn (decl))
-    flist = decl;
-
   if (! old_value)
     ;
   else if (is_overloaded_fn (old_value))
     {
-      if (flist)
-	/* It's OK to use functions from a base when there are functions with
-	   the same name already present in the current class.  */;
-      else
-	{
-	  error_at (DECL_SOURCE_LOCATION (using_decl), "%qD invalid in %q#T "
-		    "because of local method %q#D with same name",
-		    using_decl, t, old_value);
-	  inform (DECL_SOURCE_LOCATION (old_value),
-		  "local method %q#D declared here", old_value);
-	  return;
-	}
+      error_at (DECL_SOURCE_LOCATION (using_decl), "%qD invalid in %q#T "
+		"because of local method %q#D with same name",
+		using_decl, t, old_value);
+      inform (DECL_SOURCE_LOCATION (old_value),
+	      "local method %q#D declared here", old_value);
+      return;
     }
   else if (!DECL_ARTIFICIAL (old_value))
     {
@@ -1357,23 +1396,17 @@  handle_using_decl (tree using_decl, tree t)
     }
 
   iloc_sentinel ils (DECL_SOURCE_LOCATION (using_decl));
+  tree access = declared_access (using_decl);
 
   /* Make type T see field decl FDECL with access ACCESS.  */
-  if (flist)
-    for (tree f : ovl_range (flist))
-      {
-	add_method (t, f, true);
-	alter_access (t, f, access);
-      }
-  else if (USING_DECL_UNRELATED_P (using_decl))
+  if (USING_DECL_UNRELATED_P (using_decl))
     {
       /* C++20 using enum can import non-inherited enumerators into class
 	 scope.  We implement that by making a copy of the CONST_DECL for which
 	 CONST_DECL_USING_P is true.  */
       gcc_assert (TREE_CODE (decl) == CONST_DECL);
 
-      auto cas = make_temp_override (current_access_specifier);
-      set_current_access_from_decl (using_decl);
+      auto cas = make_temp_override (current_access_specifier, access);
       tree copy = copy_decl (decl);
       DECL_CONTEXT (copy) = t;
       DECL_ARTIFICIAL (copy) = true;
@@ -7672,18 +7705,8 @@  finish_struct (tree t, tree attributes)
     {
       tree x;
 
-      /* We need to add the target functions of USING_DECLS, so that
-	 they can be found when the using declaration is not
-	 instantiated yet.  */
       for (x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
-	if (TREE_CODE (x) == USING_DECL)
-	  {
-	    tree fn = strip_using_decl (x);
-  	    if (OVL_P (fn))
-	      for (lkp_iterator iter (fn); iter; ++iter)
-		add_method (t, *iter, true);
-	  }
-	else if (DECL_DECLARES_FUNCTION_P (x))
+	if (DECL_DECLARES_FUNCTION_P (x))
 	  {
 	    DECL_IN_AGGR_P (x) = false;
 	    if (DECL_VIRTUAL_P (x))
@@ -7700,14 +7723,17 @@  finish_struct (tree t, tree attributes)
 	 lookup not to fail or recurse into bases.  This isn't added
 	 to the template decl list so we drop this at instantiation
 	 time.  */
-      tree ass_op = build_lang_decl (USING_DECL, assign_op_identifier,
-				     NULL_TREE);
-      DECL_CONTEXT (ass_op) = t;
-      USING_DECL_SCOPE (ass_op) = t;
-      DECL_DEPENDENT_P (ass_op) = true;
-      DECL_ARTIFICIAL (ass_op) = true;
-      DECL_CHAIN (ass_op) = TYPE_FIELDS (t);
-      TYPE_FIELDS (t) = ass_op;
+      if (!get_class_binding_direct (t, assign_op_identifier, false))
+	{
+	  tree ass_op = build_lang_decl (USING_DECL, assign_op_identifier,
+					 NULL_TREE);
+	  DECL_CONTEXT (ass_op) = t;
+	  USING_DECL_SCOPE (ass_op) = t;
+	  DECL_DEPENDENT_P (ass_op) = true;
+	  DECL_ARTIFICIAL (ass_op) = true;
+	  DECL_CHAIN (ass_op) = TYPE_FIELDS (t);
+	  TYPE_FIELDS (t) = ass_op;
+	}
 
       TYPE_SIZE (t) = bitsize_zero_node;
       TYPE_SIZE_UNIT (t) = size_zero_node;
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index 93c4eb7193b..323f96bcd24 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -1715,7 +1715,6 @@  static void consider_binding_level (tree name,
 				    cp_binding_level *lvl,
 				    bool look_within_fields,
 				    enum lookup_name_fuzzy_kind kind);
-static void diagnose_name_conflict (tree, tree);
 
 /* ADL lookup of NAME.  FNS is the result of regular lookup, and we
    don't add duplicates to it.  ARGS is the vector of call
@@ -2711,9 +2710,13 @@  supplement_binding (cxx_binding *binding, tree decl)
   return ok;
 }
 
-/* Diagnose a name conflict between DECL and BVAL.  */
+/* Diagnose a name conflict between DECL and BVAL.
 
-static void
+   This is non-static so maybe_push_used_methods can use it and avoid changing
+   the diagnostic for inherit/using4.C; otherwise it should not be used from
+   outside this file.  */
+
+void
 diagnose_name_conflict (tree decl, tree bval)
 {
   if (TREE_CODE (decl) == TREE_CODE (bval)
@@ -5480,13 +5483,18 @@  push_class_level_binding (tree name, tree x)
 	       && DECL_DEPENDENT_P (bval))
 	return true;
       else if (TREE_CODE (decl) == USING_DECL
+	       && DECL_DEPENDENT_P (decl)
 	       && OVL_P (target_bval))
+	/* The new dependent using beats an old overload.  */
 	old_decl = bval;
       else if (TREE_CODE (bval) == USING_DECL
+	       && DECL_DEPENDENT_P (bval)
 	       && OVL_P (target_decl))
-	old_decl = bval;
+	/* The old dependent using beats a new overload.  */
+	return true;
       else if (OVL_P (target_decl)
 	       && OVL_P (target_bval))
+	/* The new overload set contains the old one.  */
 	old_decl = bval;
 
       if (old_decl && binding->scope == class_binding_level)
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index d5565ebe02d..a7f6449dafd 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -3549,8 +3549,8 @@  finish_member_declaration (tree decl)
     }
 
   if (TREE_CODE (decl) == USING_DECL)
-    /* For now, ignore class-scope USING_DECLS, so that debugging
-       backends do not see them. */
+    /* Avoid debug info for class-scope USING_DECLS for now, we'll
+       call cp_emit_debug_info_for_using later. */
     DECL_IGNORED_P (decl) = 1;
 
   /* Check for bare parameter packs in the non-static data member
@@ -3578,6 +3578,7 @@  finish_member_declaration (tree decl)
   /* Enter the DECL into the scope of the class, if the class
      isn't a closure (whose fields are supposed to be unnamed).  */
   else if (CLASSTYPE_LAMBDA_EXPR (current_class_type)
+	   || maybe_push_used_methods (decl)
 	   || pushdecl_class_level (decl))
     add = true;
 
diff --git a/gcc/testsuite/g++.dg/cpp0x/pr85070.C b/gcc/testsuite/g++.dg/cpp0x/pr85070.C
index a8b1c5013b6..5083cbd4940 100644
--- a/gcc/testsuite/g++.dg/cpp0x/pr85070.C
+++ b/gcc/testsuite/g++.dg/cpp0x/pr85070.C
@@ -4,10 +4,10 @@  struct A;
 
 struct B
 {
-  constexpr A & operator= (const A &);  // { dg-warning "used" "" { target c++14 } }
+  constexpr A & operator= (const A &);
 };
 
-struct A : B  // { dg-error "cannot be overloaded" "" { target c++14 } }
+struct A : B
 {
   using B::operator=;
 } a { a = a };
diff --git a/gcc/testsuite/g++.dg/lookup/using66a.C b/gcc/testsuite/g++.dg/lookup/using66a.C
new file mode 100644
index 00000000000..84ddd87dcd2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/using66a.C
@@ -0,0 +1,22 @@ 
+// PR c++/92918
+// { dg-do compile { target c++11 } }
+
+struct Base03
+{
+    static void impl();
+};
+
+struct ThisDoesNotCompileOnGCC : Base03
+{
+    using Base03::impl;
+    static int impl(char const *);
+
+    auto f(const char *t) const
+    -> decltype(impl(t))
+    {
+        return impl(t);
+    }
+};
+
+ThisDoesNotCompileOnGCC t;
+int i = t.f("42");
diff --git a/gcc/testsuite/g++.dg/lookup/using67.C b/gcc/testsuite/g++.dg/lookup/using67.C
new file mode 100644
index 00000000000..310d4ee9fd9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/using67.C
@@ -0,0 +1,20 @@ 
+// PR c++/104476
+// { dg-do compile { target c++11 } }
+
+struct A {
+  static void f();
+  static void f2();
+};
+
+struct B : A
+{
+  static void f(int);
+  using A::f;
+  auto g() -> decltype(f());
+  auto ga() -> decltype(f(2));
+
+  using A::f2;
+  static void f2(int);
+  auto g2() -> decltype(f2());
+  auto g2a() -> decltype(f2(2));
+};