>
> The parens are to help the editor line up the last line properly. If
> the subexpression fits on one line, the parens aren't needed.
Ahhhh okay.
No; "compile" means translate from C++ to assembly, "assemble" means
> that, plus call the assembler to produce an object file. Though since
> compilation errors out, assembling never happens.
lol I was being dumb and thinking it was the other way around. I will
change it.
You could bring back your lookup from the earlier patch and look
> through the result to see if the function we're complaining about is
> part of what the particular using-decl brings in.
I will give that a try.
I think you want
> SAME_BINFO_TYPE_P (context_for_name_lookup (decl),
> BINFO_TYPE (parent_binfo))
Am I reading this wrong then? :
/* Compare a BINFO_TYPE with another type for equality.
For a binfo, this is functionally equivalent to using same_type_p, but
measurably faster.
At least one of the arguments must be a BINFO_TYPE. The other can be a
BINFO_TYPE
or a regular type. If BINFO_TYPE(T) ever stops being the main variant of
the class the
binfo is for, this macro must change. */
#define SAME_BINFO_TYPE_P(A, B) ((A) == (B))
That leads me to believe that arguments A and B can be: BINFO, BINFO ... or
BINFO, TYPE ... or TYPE, BINFO. If so I don't see why this doesn't work:
(SAME_BINFO_TYPE_P (context_for_name_lookup (decl), parent_binfo))
and
(SAME_BINFO_TYPE_P (TYPE_BINFO (context_for_name_lookup (decl)),
parent_binfo))
Unless "BINFO_TYPE" is actually just how you refer to a regular class type
and not a BINFO. Seems a bit confusing to me.
This line needs one more space.
Oh I see ... so second line's arguments need to line up with the first
argument, not the first (.
I will send over a new patch later/tomorrow.
Anthony
On Tue, 9 Feb 2021 at 04:55, Jason Merrill <ja...@redhat.com> wrote:
> On 2/5/21 12:28 PM, Anthony Sharp wrote:
> > Hi Marek,
> >
> >>> + if ((TREE_CODE (parent_field) == USING_DECL)
> >>
> >> This first term doesn't need to be wrapped in ().
> >
> > I normally wouldn't use the (), but I think that's how Jason likes it
> > so that's why I did it. I guess it makes it more readable.
>
> Ah, no, I don't see any need for the extra () there. When I asked for
> added parens previously:
>
> >> + if (parent_binfo != NULL_TREE
> >> + && context_for_name_lookup (decl)
> >> + != BINFO_TYPE (parent_binfo))
> >
> > Here you want parens around the second != expression and its != token
> > aligned with "context"
>
> The parens are to help the editor line up the last line properly. If
> the subexpression fits on one line, the parens aren't needed.
>
> >> We usually use dg-do compile.
> >
> > True, but wouldn't that technically be slower? I would agree if it
> > were more than just error-handling code.
>
> No; "compile" means translate from C++ to assembly, "assemble" means
> that, plus call the assembler to produce an object file. Though since
> compilation errors out, assembling never happens.
>
> > + if ((TREE_CODE (parent_field) == USING_DECL)
> > + && TREE_PRIVATE (parent_field)
> > + && (DECL_NAME (decl) == DECL_NAME (parent_field)))
> > + return parent_field;
>
> Following our discussion about an earlier revision, this will often
> produce the right using-declaration, but might not if two functions of
> the same name are imported from different bases. If I split the
> using-declaration in using9.C into two, i.e.
>
> > using A2::i; // { dg-message "declared" }
>
> > using A::i;
>
> then I get
>
> > wa.ii: In member function ‘void C::f()’:
> > wa.ii:28:7: error: ‘int A::i()’ is private within this context
> > 28 | i();
> > | ^
> > wa.ii:20:13: note: declared private here
> > 20 | using A2::i;
>
> pointing out the wrong using-declaration because it happens to be first.
> You could bring back your lookup from the earlier patch and look
> through the result to see if the function we're complaining about is
> part of what the particular using-decl brings in.
>
> > I tried both:
> > (SAME_BINFO_TYPE_P (context_for_name_lookup (decl), parent_binfo))
> > and
> > (SAME_BINFO_TYPE_P (TYPE_BINFO (context_for_name_lookup (decl)),
> parent_binfo))
>
> I think you want
>
> SAME_BINFO_TYPE_P (context_for_name_lookup (decl),
> BINFO_TYPE (parent_binfo))
>
> i.e. just change same_type_p to SAME_BINFO_TYPE_P.
>
> > tree parent_binfo = get_parent_with_private_access (decl,
> > -
> basetype_path);
> > +
> basetype_path);
>
> This line was indented properly before, and is changed to be indented
> one space too far.
>
> > + diag_location = get_class_access_diagnostic_decl
> (parent_binfo,
> > +
> diag_decl);
>
> This line needs one more space.
>
> > complain_about_access (decl, diag_decl, diag_location, true,
> > - parent_access);
> > + access_failure_reason);
>
> This line, too.
>
> > +};
> > \ No newline at end of file
>
> Let's add a newline.
>
> Jason
>
>
