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

Reply via email to