Re: [PATCH] c++: private inheritance access diagnostics fix [PR17314]

[Jason Merrill <jason@redhat.com>]

On 1/22/21 3:07 PM, Anthony Sharp wrote:
> Hi Jason,
> 
> Thanks for getting back to me so quickly.
> 
>  > Why two gcc-comit-mklog?  That would generate the log entries twice.
> 
> It did in fact generate the log entries twice, but I deleted out the 
> second copy. Perhaps it would have made more sense to do git commit 
> --amend instead.
> 
>  > Instead of making access_in_type non-static, let's defiine
>  > get_parent_with_private_access in search.c and declare it in cp-tree.h
>  > (with the declarations of nearby search.c functions).
> 
> Done.
> 
>  > Only one 'n' in inaccessible.
> 
> Oops!
> 
>     Subsequent lines of a comment should be indented to line up with the
>     first line.  This applies to all your multi-line comments.
> 
> 
> My bad, hopefully fixed now.
> 
>     Don't change the indentation of these blocks; in the GNU coding style
>     the { } are indented two spaces from the if.
> 
> 
> I think I see what you mean; I forgot to indent the { } (and therefore 
> also everything within it, by two spaces). Hopefully fixed.
> 
>     The new line of arguments should be indented to line up with the
>     first one.
> 
> 
> Fixed I think.
> 
> Please find attached the latest patch version with all these changes. 
> git gcc-verify returns no problems and check_GNU_style.sh returns only 
> false positives. Source builds fine. To be super safe I re-cloned the 
> source and did git apply with the patch and it built and worked just 
> fine, and hopefully I haven't missed anything.
> 
> Thanks again for your help.

> Subject: [PATCH] This patch fixes PR17314. Previously, when class C attempted
>  to access member a declared in class A through class B, where class B
>  privately inherits from A and class C inherits from B, GCC would correctly
>  report an access violation, but would erroneously report that the reason was
>  because a was "protected", when in fact, from the point of view of class C,
>  it was really "private". This patch updates the diagnostics code to generate
>  more correct errors in cases of failed inheritance such as these.

The first line of the commit message should be the subject line for the 
patch, i.e. "c++: private inheritance access diagnostics fix [PR17314]", 
then a blank line, then the rationale.

> +	  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"

> +	  complain_about_access (decl, diag_decl, diag_location, true,
> +				  parent_access);
...
> +	  complain_about_access (afi.get_decl (), afi.get_diag_decl (),
> +				  afi.get_diag_decl (), false, ak_none);

In both these calls, the second line is indented one space too far.

Jason