* [PATCH] c++: extraneous access error with ambiguous lookup [PR103177]
@ 2022-03-14 17:24 Patrick Palka
2022-03-14 21:58 ` Jason Merrill
0 siblings, 1 reply; 2+ messages in thread
From: Patrick Palka @ 2022-03-14 17:24 UTC (permalink / raw)
To: gcc-patches
When a lookup is ambiguous, lookup_member still attempts to check
access of the first member found before diagnosing the ambiguity and
erroring out, which may cause us to issue an extraneous access error
in case of ambiguous lookup 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 possible error
that could happen during lookup_field_r is an ambiguity error, we might
as well hardcode that in lookup_member instead and remove the unneeded
lookup_field_info::errstr.
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?
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 2b8210408fb..85e3e7cb487 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 00000000000..45e8effba43
--- /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.35.1.455.g1a4874565f
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] c++: extraneous access error with ambiguous lookup [PR103177]
2022-03-14 17:24 [PATCH] c++: extraneous access error with ambiguous lookup [PR103177] Patrick Palka
@ 2022-03-14 21:58 ` Jason Merrill
0 siblings, 0 replies; 2+ messages in thread
From: Jason Merrill @ 2022-03-14 21:58 UTC (permalink / raw)
To: Patrick Palka, gcc-patches
On 3/14/22 13:24, Patrick Palka wrote:
> When a lookup is ambiguous, lookup_member still attempts to check
> access of the first member found before diagnosing the ambiguity and
> erroring out, which may cause us to issue an extraneous access error
> in case of ambiguous lookup 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 possible error
> that could happen during lookup_field_r is an ambiguity error, we might
> as well hardcode that in lookup_member instead and remove the unneeded
> lookup_field_info::errstr.
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?
OK.
> 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 2b8210408fb..85e3e7cb487 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 00000000000..45e8effba43
> --- /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 }
> +}
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-03-14 21:58 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-14 17:24 [PATCH] c++: extraneous access error with ambiguous lookup [PR103177] Patrick Palka
2022-03-14 21:58 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).