On 2/9/21 6:18 AM, Anthony Sharp wrote:
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.
The comment is pretty conusing, I agree. But what it's saying is that
both arguments must be types, and one of those types must be BINFO_TYPE
(some_binfo).
Your first line doesn't work because you're comparing a type and a
binfo. The second one doesn't work because you're comparing the binfo
for a most-derived object of the type to the binfo for a base subobject
of the same type, and those are different, because binfos represent
nodes in inheritance graphs.
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
<mailto: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
