On Fri, Aug 09, 2024 at 11:03:24AM +1000, Nathaniel Shead wrote:
> On Thu, Aug 08, 2024 at 03:16:24PM -0400, Marek Polacek wrote:
> > On Thu, Aug 08, 2024 at 09:13:05AM +1000, Nathaniel Shead wrote:
> > > diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc
> > > index 6c22ff55b46..03c19e4a7e4 100644
> > > --- a/gcc/cp/error.cc
> > > +++ b/gcc/cp/error.cc
> > > @@ -4782,12 +4782,14 @@ qualified_name_lookup_error (tree scope, tree
> > > name,
> > > scope);
> > > else if (TREE_CODE (decl) == TREE_LIST)
> > > {
> > > + auto_diagnostic_group d;
> > > error_at (location, "reference to %<%T::%D%> is ambiguous",
> > > scope, name);
> > > print_candidates (decl);
> > > }
> > > else
> > > {
> > > + auto_diagnostic_group d;
> > > name_hint hint;
> > > if (SCOPED_ENUM_P (scope) && TREE_CODE (name) == IDENTIFIER_NODE)
> > > hint = suggest_alternative_in_scoped_enum (name, scope);
> >
> > I don't see why we need the second a_d_d here.
> >
>
> The 'suggest_alternative_in_scoped_enum' call can register an 'inform'
> to be called in 'name_hint's destructor, specifically contained within
> name_hint::m_deferred (a deferred_diagnostic).
>
> Most other uses of 'name_hint' that I could find seemed to be correctly
> grouped already.
>
> > > @@ -3534,6 +3536,7 @@ finish_class_member_access_expr (cp_expr object,
> > > tree name, bool template_p,
> > > else
> > > {
> > > /* Look up the member. */
> > > + auto_diagnostic_group d;
> > > access_failure_info afi;
> > > if (processing_template_decl)
> > > /* Even though this class member access expression is at this
> >
> > I don't quite see why we need it here, either.
> >
>
> A little later on in this block there's afi.maybe_suggest_accessor,
> which emits an 'inform' with a fixit suggesting how to access the
> member, which I feel should probably be part of the same group as the
> error emitted from the 'lookup_member' call.
>
> But I suppose this should be grouped more clearly, e.g.
>
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -3534,7 +3536,6 @@ finish_class_member_access_expr (cp_expr object, tree
> name, bool template_p,
> else
> {
> /* Look up the member. */
> - access_failure_info afi;
> if (processing_template_decl)
> /* Even though this class member access expression is at this
> point not dependent, the member itself may be dependent, and
> @@ -3543,12 +3544,18 @@ finish_class_member_access_expr (cp_expr object, tree
> name, bool template_p,
> ahead of time here; we're going to redo this member lookup at
> instantiation time anyway. */
> push_deferring_access_checks (dk_no_check);
Actually it feels wrong for the p_d_a_c to not be part of this group;
here's a better incremental patch on my first submission:
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index d4fc848bfa1..e4e260645f6 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -3536,22 +3536,24 @@ finish_class_member_access_expr (cp_expr object, tree
name, bool template_p,
else
{
/* Look up the member. */
- auto_diagnostic_group d;
- access_failure_info afi;
- if (processing_template_decl)
- /* Even though this class member access expression is at this
- point not dependent, the member itself may be dependent, and
- we must not potentially push a access check for a dependent
- member onto TI_DEFERRED_ACCESS_CHECKS. So don't check access
- ahead of time here; we're going to redo this member lookup at
- instantiation time anyway. */
- push_deferring_access_checks (dk_no_check);
- member = lookup_member (access_path, name, /*protect=*/1,
- /*want_type=*/false, complain,
- &afi);
- if (processing_template_decl)
- pop_deferring_access_checks ();
- afi.maybe_suggest_accessor (TYPE_READONLY (object_type));
+ {
+ auto_diagnostic_group d;
+ access_failure_info afi;
+ if (processing_template_decl)
+ /* Even though this class member access expression is at this
+ point not dependent, the member itself may be dependent, and
+ we must not potentially push a access check for a dependent
+ member onto TI_DEFERRED_ACCESS_CHECKS. So don't check access
+ ahead of time here; we're going to redo this member lookup at
+ instantiation time anyway. */
+ push_deferring_access_checks (dk_no_check);
+ member = lookup_member (access_path, name, /*protect=*/1,
+ /*want_type=*/false, complain,
+ &afi);
+ if (processing_template_decl)
+ pop_deferring_access_checks ();
+ afi.maybe_suggest_accessor (TYPE_READONLY (object_type));
+ }
if (member == NULL_TREE)
{
if (dependentish_scope_p (object_type))
Nathaniel
