c++: Private parent access check for using decls [PR19377]

This bug was already mostly fixed by the patch for PR17314. This
patch continues that by ensuring that where a using decl is used,
causing an access failure to a child class because the using decl is
private, the compiler correctly points to the using decl as the
source of the problem.

gcc/cp/ChangeLog:

2021-03-10  Anthony Sharp  <anthonysharp15@gmail.com>

	* semantics.c (get_class_access_diagnostic_decl): New
	function that examines special cases when a parent
	class causes a private access failure.
	(enforce_access): Slightly modified to call function
	above.

gcc/testsuite/ChangeLog:

2021-03-10  Anthony Sharp  <anthonysharp15@gmail.com>

	* g++.dg/cpp1z/using9.C: New using decl test.

Co-authored-by: Jason Merrill <jason@redhat.com>

2 files changed, 132 insertions, 18 deletions

diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 30dd206c899..b02596f73bd 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -256,6 +256,72 @@ pop_to_parent_deferring_access_checks (void)
     }
 }
 
+/* Called from enforce_access.  A class has attempted (but failed) to access
+   DECL.  It is already established that a baseclass of that class,
+   PARENT_BINFO, has private access to DECL.  Examine certain special cases
+   to find a decl that accurately describes the source of the problem.  If
+   none of the special cases apply, simply return DECL as the source of the
+   problem.  */
+
+static tree
+get_class_access_diagnostic_decl (tree parent_binfo, tree decl)
+{
+  /* When a class is denied access to a decl in a baseclass, most of the
+     time it is because the decl itself was declared as private at the point
+     of declaration.
+
+     However, in C++, there are (at least) two situations in which a decl
+     can be private even though it was not originally defined as such.
+     These two situations only apply if a baseclass had private access to
+     DECL (this function is only called if that is the case).  */
+
+  /* We should first check whether the reason the parent had private access
+     to DECL was simply because DECL was created and declared as private in
+     the parent.  If it was, then DECL is definitively the source of the
+     problem.  */
+  if (SAME_BINFO_TYPE_P (context_for_name_lookup (decl),
+			 BINFO_TYPE (parent_binfo)))
+    return decl;
+
+  /* 1.  If the "using" keyword is used to inherit DECL within the parent,
+     this may cause DECL to be private, so we should return the using
+     statement as the source of the problem.
+
+     Scan the fields of PARENT_BINFO and see if there are any using decls.  If
+     there are, see if they inherit DECL.  If they do, that's where DECL must
+     have been declared private.  */
+
+  for (tree parent_field = TYPE_FIELDS (BINFO_TYPE (parent_binfo));
+       parent_field;
+       parent_field = DECL_CHAIN (parent_field))
+    /* Not necessary, but also check TREE_PRIVATE for the sake of
+       eliminating obviously non-relevant using decls.  */
+    if (TREE_CODE (parent_field) == USING_DECL
+	&& TREE_PRIVATE (parent_field))
+      {
+	tree decl_stripped = strip_using_decl (parent_field);
+
+	/* The using statement might be overloaded.  If so, we need to
+	   check all of the overloads.  */
+	for (ovl_iterator iter (decl_stripped); iter; ++iter)
+	  /* If equal, the using statement inherits DECL, and so is the
+	     source of the access failure, so return it.  */
+	  if (*iter == decl)
+	    return parent_field;
+      }
+
+  /* 2.  If DECL was privately inherited by the parent class, then DECL will
+     be inaccessible, even though it may originally have been accessible to
+     deriving classes.  In that case, the fault lies with the parent, since it
+     used a private inheritance, so we return the parent as the source of the
+     problem.
+
+     Since this is the last check, we just assume it's true.  At worst, it
+     will simply point to the class that failed to give access, which is
+     technically true.  */
+  return TYPE_NAME (BINFO_TYPE (parent_binfo));
+}
+
 /* If the current scope isn't allowed to access DECL along
    BASETYPE_PATH, give an error, or if we're parsing a function or class
    template, defer the access check to be performed at instantiation time.
@@ -317,34 +383,33 @@ enforce_access (tree basetype_path, tree decl, tree diag_decl,
 	diag_decl = strip_inheriting_ctors (diag_decl);
       if (complain & tf_error)
 	{
-	  /* We will usually want to point to the same place as
-	     diag_decl but not always.  */
+	  access_kind access_failure_reason = ak_none;
+
+	  /* By default, using the decl as the source of the problem will
+	     usually give correct results.  */
 	  tree diag_location = diag_decl;
-	  access_kind parent_access = ak_none;
 
-	  /* See if any of BASETYPE_PATH's parents had private access
-	     to DECL.  If they did, that will tell us why we don't.  */
+	  /* However, if a parent of BASETYPE_PATH had private access to decl,
+	     then it actually might be the case that the source of the problem
+	     is not DECL.  */
 	  tree parent_binfo = get_parent_with_private_access (decl,
 							      basetype_path);
 
-	  /* If a parent had private access, then the diagnostic
-	     location DECL should be that of the parent class, since it
-	     failed to give suitable access by using a private
-	     inheritance.  But if DECL was actually defined in the parent,
-	     it wasn't privately inherited, and so we don't need to do
-	     this, and complain_about_access will figure out what to
-	     do.  */
-	  if (parent_binfo != NULL_TREE
-	      && (context_for_name_lookup (decl)
-		  != BINFO_TYPE (parent_binfo)))
+	  /* So if a parent did have private access, then we need to do
+	     special checks to obtain the best diagnostic location decl.  */
+	  if (parent_binfo != NULL_TREE)
 	    {
-	      diag_location = TYPE_NAME (BINFO_TYPE (parent_binfo));
-	      parent_access = ak_private;
+	      diag_location = get_class_access_diagnostic_decl (parent_binfo,
+								diag_decl);
+
+	      /* We also at this point know that the reason access failed was
+		 because decl was private.  */
+	      access_failure_reason = ak_private;
 	    }
 
 	  /* Finally, generate an error message.  */
 	  complain_about_access (decl, diag_decl, diag_location, true,
-				 parent_access);
+				 access_failure_reason);
 	}
       if (afi)
 	afi->record_access_failure (basetype_path, decl, diag_decl);
diff --git a/gcc/testsuite/g++.dg/cpp1z/using9.C b/gcc/testsuite/g++.dg/cpp1z/using9.C
new file mode 100644
index 00000000000..98e36babb01
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/using9.C
@@ -0,0 +1,49 @@
+/* { dg-do compile { target c++17 } } */
+// Created for c++ PR19377
+
+class A2
+{
+  protected:
+  int separate(int a);
+  int separate(int a, int b);
+  int separate(int a, int b, int c);
+  int comma(int a);
+  int alone;
+};
+
+class A1
+{
+  protected:
+  int separate();
+  int comma();
+};
+
+class A3
+{
+  protected:
+  int comma(int a, int b);
+};
+
+class B:private A3, private A1, private A2
+{
+  // Using decls in a comma-separated list.
+  using A2::comma, A3::comma, A1::comma;  // { dg-message "declared" }
+  // Separate using statements.
+  using A2::separate; // { dg-message "declared" }
+  using A1::separate; // { dg-message "declared" }
+  // No ambiguity, just for the sake of it.
+  using A2::alone; // { dg-message "declared" }
+};
+
+class C:public B
+{
+  void f()
+  {
+    comma(); // { dg-error "private" }
+    separate(); // { dg-error "private" }
+    separate(1); // { dg-error "private" }
+    separate(1, 2); // { dg-error "private" }
+    separate(1, 2, 3); // { dg-error "private" }
+    alone = 5; // { dg-error "private" }
+  }
+};