From ffe9c0a0d3564a6083ea6194eb3374a89c29c085 Mon Sep 17 00:00:00 2001
From: Patrick Palka <ppalka@redhat.com>
Date: Tue, 15 Mar 2022 08:50:24 -0400
Subject: [PATCH] c++: extraneous access error with ambiguous lookup [PR103177]

When a lookup is ambiguous, lookup_member still attempts to check
access of the first member found before diagnosing the ambiguity and
propagating the error, and this may cause us to issue an extraneous
access error as in the testcase below (for B1::foo).

This patch fixes this by swapping the order of the ambiguity and access
checks within lookup_member.  In passing, since the only thing that could
go wrong during lookup_field_r is ambiguity, we might as well hardcode
that in lookup_member and get rid of lookup_field_info::errstr.

	PR c++/103177

gcc/cp/ChangeLog:

	* search.cc (lookup_field_info::errstr): Remove this data
	member.
	(lookup_field_r): Don't set errstr.
	(lookup_member): Check ambiguity before checking access.
	Simplify accordingly after errstr removal.  Exit early upon
	error or empty result.

gcc/testsuite/ChangeLog:

	* g++.dg/lookup/ambig6.C: New test.
---
 gcc/cp/search.cc                     | 37 ++++++++++++++++--------------------
 gcc/testsuite/g++.dg/lookup/ambig6.C | 18 ++++++++++++++++++
 2 files changed, 34 insertions(+), 21 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/lookup/ambig6.C

diff --git a/gcc/cp/search.cc b/gcc/cp/search.cc
index 2b82104..85e3e7c 100644
--- a/gcc/cp/search.cc
+++ b/gcc/cp/search.cc
@@ -941,8 +941,6 @@ struct lookup_field_info {
   tree ambiguous;
   /* If nonzero, we are looking for types, not data members.  */
   int want_type;
-  /* If something went wrong, a message indicating what.  */
-  const char *errstr;
 };
 
 /* True for a class member means that it is shared between all objects
@@ -1055,7 +1053,6 @@ lookup_field_r (tree binfo, void *data)
 	  /* Add the new value.  */
 	  lfi->ambiguous = tree_cons (NULL_TREE, nval, lfi->ambiguous);
 	  TREE_TYPE (lfi->ambiguous) = error_mark_node;
-	  lfi->errstr = G_("request for member %qD is ambiguous");
 	}
     }
   else
@@ -1127,8 +1124,6 @@ lookup_member (tree xbasetype, tree name, int protect, bool want_type,
      checks.  Whereas rval is only set if a proper (not hidden)
      non-function member is found.  */
 
-  const char *errstr = 0;
-
   if (name == error_mark_node
       || xbasetype == NULL_TREE
       || xbasetype == error_mark_node)
@@ -1172,7 +1167,6 @@ lookup_member (tree xbasetype, tree name, int protect, bool want_type,
   rval_binfo = lfi.rval_binfo;
   if (rval_binfo)
     type = BINFO_TYPE (rval_binfo);
-  errstr = lfi.errstr;
 
   /* If we are not interested in ambiguities, don't report them;
      just return NULL_TREE.  */
@@ -1187,6 +1181,19 @@ lookup_member (tree xbasetype, tree name, int protect, bool want_type,
 	protect = 0;
     }
 
+  if (protect == 1 && lfi.ambiguous)
+    {
+      if (complain & tf_error)
+	{
+	  error ("request for member %qD is ambiguous", name);
+	  print_candidates (lfi.ambiguous);
+	}
+      return error_mark_node;
+    }
+
+  if (!rval)
+    return NULL_TREE;
+
   /* [class.access]
 
      In the case of overloaded function names, access control is
@@ -1206,8 +1213,7 @@ lookup_member (tree xbasetype, tree name, int protect, bool want_type,
 
     only the first call to "f" is valid.  However, if the function is
     static, we can check.  */
-  if (rval && protect 
-      && !really_overloaded_fn (rval))
+  if (protect && !really_overloaded_fn (rval))
     {
       tree decl = is_overloaded_fn (rval) ? get_first_fn (rval) : rval;
       decl = strip_using_decl (decl);
@@ -1216,21 +1222,10 @@ lookup_member (tree xbasetype, tree name, int protect, bool want_type,
 	  && !DECL_NONSTATIC_MEMBER_FUNCTION_P (decl)
 	  && !perform_or_defer_access_check (basetype_path, decl, decl,
 					     complain, afi))
-	rval = error_mark_node;
-    }
-
-  if (errstr && protect)
-    {
-      if (complain & tf_error)
-	{
-	  error (errstr, name, type);
-	  if (lfi.ambiguous)
-	    print_candidates (lfi.ambiguous);
-	}
-      rval = error_mark_node;
+	return error_mark_node;
     }
 
-  if (rval && is_overloaded_fn (rval)
+  if (is_overloaded_fn (rval)
       /* Don't use a BASELINK for class-scope deduction guides since
 	 they're not actually member functions.  */
       && !dguide_name_p (name))
diff --git a/gcc/testsuite/g++.dg/lookup/ambig6.C b/gcc/testsuite/g++.dg/lookup/ambig6.C
new file mode 100644
index 0000000..eeef45e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/ambig6.C
@@ -0,0 +1,18 @@
+// PR c++/103177
+
+struct B1 {
+private:
+  static int foo();
+};
+
+struct B2 {
+  int foo();
+};
+
+struct D : B1, B2 { };
+
+void f() {
+  D d;
+  d.foo(); // { dg-error "ambiguous" }
+           // { dg-bogus "private" "" { target *-*-* } .-1 }
+}
-- 
2.7.4