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);
- 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;
+ 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))
> > @@ -10384,6 +10401,8 @@ convert_for_assignment (tree type, tree rhs,
> > {
> > if (complain & tf_error)
> > {
> > + auto_diagnostic_group d;
> > +
> > /* If the right-hand side has unknown type, then it is an
> > overloaded function. Call instantiate_type to get error
> > messages. */
> > @@ -10406,7 +10425,6 @@ convert_for_assignment (tree type, tree rhs,
> > (rhs_loc,
> > has_loc ? &label : NULL,
> > has_loc ? highlight_colors::percent_h : NULL);
> > - auto_diagnostic_group d;
>
> Oh I see, it was supposed to be in the outer block. OK.
>
> The patch looks good to me, thanks.
>
> Marek
>
Thanks, I'll wait for final approval from someone for once I've finished
bootstrap+regtest with the above adjustment.
Nathaniel
