* [PATCH] c++: private inheritance access diagnostics fix [PR17314]
@ 2021-01-05 14:24 Anthony Sharp
2021-01-07 21:01 ` Jason Merrill
0 siblings, 1 reply; 10+ messages in thread
From: Anthony Sharp @ 2021-01-05 14:24 UTC (permalink / raw)
To: gcc-patches
This patch fixes PR17314 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=17314).
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 "private". This patch updates the
diagnostics code to generate more correct errors in cases of failed
inheritance such as these.
The reason this bug happened was because GCC was examining the
declared access of decl, instead of looking at it in the context of class
inheritance.
----------- COMMENTS -----------
This is my first GCC patch ever so there is probably something I have done
very wrong. Please let me know :) The thought of my code being scrutinised
by people with PhDs and doctorates is quite frankly terrifying.
Note that since it is a new year I had to make a new changelog file so the
diff for the patch might be slightly off.
There was no need to add additional regression tests since it was adequate
to simply change some of the regression tests that were there originally
(all the patch changes is the informative message telling the user where
a decl was defined as private).
----------- REGRESSION ANALYSIS -----------
No regressions reported.
G++ (CLEAN) RESULTS
# of expected passes 202879
# of unexpected failures 1
# of expected failures 988
# of unsupported tests 8654
GCC (CLEAN) RESULTS
# of expected passes 163377
# of unexpected failures 94
# of unexpected successes 37
# of expected failures 915
# of unsupported tests 2530
G++ (PR17314 PATCHED) RESULTS
# of expected passes 202871
# of unexpected failures 1
# of expected failures 988
# of unsupported tests 8654
GCC (PR17314 PATCHED) RESULTS
# of expected passes 163377
# of unexpected failures 94
# of unexpected successes 37
# of expected failures 915
# of unsupported tests 2530
When I build and make -k check -j 6 on the patched source it reports
202871 passes (8 fewer), although the FAILs do not increase. I am not 100%
sure why this happens since I have not removed any testcases, only edited a
few, but I think this happens because in files like dr142.c I removed more
output checks than I added. make -k check -j 6 also returns error 2
sometimes, although there are no obvious errors or warnings in the logs
explaining why. Probably harmless?
----------- BUILD REPORT -----------
GCC builds normally on x86_64-pc-linux-gnu for x86_64-pc-linux-gnu using
make -j 6. I didn't see it necessary to test on other build targets since the
patch only affects the C++ front end and so functionality is unlikely
to differ between platforms.
The compile log reports:
Comparing stages 2 and 3
warning: gcc/cc1obj-checksum.o differs
Comparison successful.
and then continues. I assume this means it was actually successful.
Index: gcc/cp/ChangeLog
from Anthony Sharp <anthonysharp15@gmail.com>
Fixes PR17314
* typeck.c (complain_about_unrecognized_member): Updated function
arguments in complain_about_access.
* call.c (complain_about_access): Altered function.
* semantics.c (get_parent_with_private_access): Added function.
(access_in_type): Added as extern function.
* search.c (access_in_type): Made function non-static so it can be
used in semantics.c.
* cp-tree.h (complain_about_access): Changed parameters of function.
Index: gcc/testsuite/ChangeLog
from Anthony Sharp <anthonysharp15@gmail.com>
Fixes PR17314
* g++.dg/lookup/scoped1.c modified testcase to run successfully with
changes.
* g++.dg/tc1/dr142.c modified testcase to run successfully with
changes.
* g++.dg/tc1/dr142.c modified testcase to run successfully with
changes.
* g++.dg/tc1/dr142.c modified testcase to run successfully with
changes.
* g++.dg/tc1/dr52.c modified testcase to run successfully with changes.
* g++.old-deja/g++.brendan/visibility6.c modified testcase to run
successfully with changes.
* g++.old-deja/g++.brendan/visibility8.c modified testcase to run
successfully with changes.
* g++.old-deja/g++.jason/access8.c modified testcase to run
successfully with changes.
* g++.old-deja/g++.law/access4.c modified testcase to run successfully
with changes.
* g++.old-deja/g++.law/visibility12.c modified testcase to run
successfully with changes.
* g++.old-deja/g++.law/visibility4.c modified testcase to run
successfully with changes.
* g++.old-deja/g++.law/visibility8.c modified testcase to run
successfully with changes.
* g++.old-deja/g++.other/access4.c modified testcase to run
successfully with changes.
Index: gcc/testsuite/g++.old-deja/g++.jason/access8.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.jason/access8.C 2020-12-31
16:51:34.000000000 +0000
+++ gcc/testsuite/g++.old-deja/g++.jason/access8.C 2021-01-03
00:22:14.969854000 +0000
@@ -4,5 +4,5 @@
// Bug: g++ forgets access decls after the definition.
-class inh { // { dg-message "" } inaccessible
+class inh {
int a;
protected:
@@ -10,5 +10,6 @@ protected:
};
-class mel : private inh {
+class mel : private inh // { dg-message "" } inaccessible
+{
protected:
int t;
Index: gcc/testsuite/g++.old-deja/g++.law/access4.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.law/access4.C 2020-12-31
16:51:34.000000000 +0000
+++ gcc/testsuite/g++.old-deja/g++.law/access4.C 2021-01-03
00:21:01.736320000 +0000
@@ -7,10 +7,11 @@
// Message-ID: <9403030024.AA04534@ses.com>
-class ForceLeafSterile { // { dg-message "" }
+class ForceLeafSterile {
friend class Sterile;
ForceLeafSterile() {} // { dg-message "" }
};
-class Sterile : private virtual ForceLeafSterile {
+class Sterile : private virtual ForceLeafSterile // { dg-message "" }
+{
public:
Sterile() {}
Index: gcc/testsuite/g++.old-deja/g++.law/visibility12.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.law/visibility12.C 2020-12-31
16:51:34.000000000 +0000
+++ gcc/testsuite/g++.old-deja/g++.law/visibility12.C 2021-01-03
00:20:24.243535000 +0000
@@ -7,9 +7,10 @@
// Message-ID: <9306300528.AA17185@coda.mel.dit.CSIRO.AU>
struct a {
- int aa; // { dg-message "" } private
+ int aa;
};
-class b : private a {
- };
+class b : private a // { dg-message "" } private
+{
+};
class c : public b {
Index: gcc/testsuite/g++.old-deja/g++.law/visibility8.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.law/visibility8.C 2020-12-31
16:51:34.000000000 +0000
+++ gcc/testsuite/g++.old-deja/g++.law/visibility8.C 2021-01-03
00:19:45.574725000 +0000
@@ -8,9 +8,10 @@
class t1 {
protected:
- int a; // { dg-message "" } protected
+ int a;
};
-class t2 : private t1 {
+class t2 : private t1 // { dg-message "" } protected
+{
public:
int b;
Index: gcc/testsuite/g++.old-deja/g++.law/visibility4.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.law/visibility4.C 2020-12-31
16:51:34.000000000 +0000
+++ gcc/testsuite/g++.old-deja/g++.law/visibility4.C 2021-01-03
00:20:06.815170000 +0000
@@ -9,8 +9,9 @@
class A {
public:
- int b; // { dg-message "" } private
+ int b;
};
-class C : private A { // NOTE WELL. private, not public
+class C : private A // { dg-message "" } private
+{
public:
int d;
Index: gcc/testsuite/g++.old-deja/g++.other/access4.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.other/access4.C 2020-12-31
16:51:34.000000000 +0000
+++ gcc/testsuite/g++.old-deja/g++.other/access4.C 2021-01-03
00:19:25.362302000 +0000
@@ -1,9 +1,9 @@
// { dg-do assemble }
-struct A { // { dg-message "" } inaccessible
+struct A {
static int i;
};
-struct B : private A { };
+struct B : private A { }; // { dg-message "" } inaccessible
struct C : public B {
Index: gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C 2020-12-31
16:51:34.000000000 +0000
+++ gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C 2021-01-03
00:19:17.918146000 +0000
@@ -6,7 +6,7 @@ class foo
{
public:
- static int y; // { dg-message "" } private
+ static int y;
};
-class foo1 : private foo
+class foo1 : private foo // { dg-message "" } private
{ };
class foo2 : public foo1
Index: gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C 2020-12-31
16:51:34.000000000 +0000
+++ gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C 2021-01-03
00:21:55.873454000 +0000
@@ -4,7 +4,7 @@ class bottom
{
public:
- int b; // { dg-message "" } private
+ int b;
};
-class middle : private bottom
+class middle : private bottom // { dg-message "" } private
{
public:
Index: gcc/testsuite/g++.dg/lookup/scoped1.C
===================================================================
--- gcc/testsuite/g++.dg/lookup/scoped1.C 2020-12-31 16:51:34.000000000 +0000
+++ gcc/testsuite/g++.dg/lookup/scoped1.C 2021-01-02 21:51:30.088674000 +0000
@@ -5,10 +5,10 @@ struct A
{
static int i1;
- int i2; // { dg-message "declared" }
+ int i2;
static void f1 ();
void f2 ();
};
-struct B: private A { };
+struct B: private A { }; // { dg-message "declared" }
struct C: public B
{
Index: gcc/testsuite/g++.dg/tc1/dr142.C
===================================================================
--- gcc/testsuite/g++.dg/tc1/dr142.C 2020-12-31 16:51:34.000000000 +0000
+++ gcc/testsuite/g++.dg/tc1/dr142.C 2021-01-02 21:54:09.839546000 +0000
@@ -3,11 +3,11 @@
// DR142: Injection-related errors in access example
-class B { // { dg-message "declared" }
+class B {
public:
- int mi; // { dg-message "declared" }
- static int si; // { dg-message "declared" }
+ int mi;
+ static int si;
};
-class D: private B {
+class D: private B { // { dg-message "declared" }
};
Index: gcc/testsuite/g++.dg/tc1/dr52.C
===================================================================
--- gcc/testsuite/g++.dg/tc1/dr52.C 2020-12-31 16:51:34.000000000 +0000
+++ gcc/testsuite/g++.dg/tc1/dr52.C 2021-01-03 13:20:08.267946000 +0000
@@ -18,9 +18,9 @@ struct B2 : B {};
struct C
-{ // { dg-message "declared" }
- void foo(void);
+{
+ void foo(void);
};
-struct D : private C {};
+struct D : private C {}; // { dg-message "declared" }
struct X: A, B1, B2, D
Index: gcc/cp/semantics.c
===================================================================
--- gcc/cp/semantics.c 2020-12-31 16:51:34.000000000 +0000
+++ gcc/cp/semantics.c 2021-01-04 20:26:51.912302840 +0000
@@ -47,4 +47,6 @@ along with GCC; see the file COPYING3.
#include "memmodel.h"
+extern access_kind access_in_type (tree type, tree decl);
+
/* There routines provide a modular interface to perform many parsing
operations. They may therefore be used during actual parsing, or
@@ -257,4 +259,39 @@ pop_to_parent_deferring_access_checks (v
}
+/* This deals with bug PR17314.
+
+ DECL is a declaration and BINFO represents a class that has attempted (but
+ failed) to access DECL.
+
+ Examine the parent binfos of BINFO and determine whether any of them had
+ private access to DECL. If they did, return the parent binfo. This helps
+ in figuring out the correct error message to show (if the parents had
+ access, it's their fault for not giving sufficient access to BINFO).
+
+ If no parents had access, return NULL_TREE. */
+
+static tree
+get_parent_with_private_access(tree decl, tree binfo)
+{
+ /* Only BINFOs should come through here. */
+ gcc_assert(TREE_CODE(binfo) == TREE_BINFO);
+
+ tree base_binfo = NULL_TREE;
+
+ /* Iterate through immediate parent classes. */
+ for (int i = 0; BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
+ {
+ /* This parent had private access. Therefore that's why BINFO can't access
+ DECL. */
+ if(access_in_type(BINFO_TYPE(base_binfo), decl) == ak_private)
+ return base_binfo;
+ }
+
+ /* None of the parents had access. Note: it's impossible for one of the
+ parents to have had public or protected access to DECL, since then
+ BINFO would have been able to access DECL too. */
+ return NULL_TREE;
+}
+
/* If the current scope isn't allowed to access DECL along
BASETYPE_PATH, give an error, or if we're parsing a function or class
@@ -315,9 +352,34 @@ enforce_access (tree basetype_path, tree
{
if (flag_new_inheriting_ctors)
- diag_decl = strip_inheriting_ctors (diag_decl);
+ diag_decl = strip_inheriting_ctors (diag_decl);
if (complain & tf_error)
- complain_about_access (decl, diag_decl, true);
+ {
+ /* We will usually want to point to the same place as diag_decl but
+ not always. */
+ tree diag_location = diag_decl;
+ access_kind parent_access = ak_none;
+
+ /* See if any of BASETYPE_PATH's parents had private access to DECL.
+ If they did, that will tell us why we don't. */
+ tree parent_binfo = get_parent_with_private_access(decl,
basetype_path);
+
+ /* If a parent had private access, then the diagnostic location DECL
+ should be that of the parent class, since it failed to give suitable
+ access by using a private inheritance. But if DECL was actually defined
+ in the parent, it wasn't privately inherited, and so we don't need to
+ do this, and complain_about_access will figure out what to do. */
+ if(parent_binfo != NULL_TREE
+ && context_for_name_lookup(decl) != BINFO_TYPE(parent_binfo))
+ {
+ diag_location = TYPE_NAME(BINFO_TYPE(parent_binfo));
+ parent_access = ak_private;
+ }
+
+ /* Finally, generate an error message. */
+ complain_about_access (decl, diag_decl, diag_location, true,
+ parent_access);
+ }
if (afi)
- afi->record_access_failure (basetype_path, decl, diag_decl);
+ afi->record_access_failure (basetype_path, decl, diag_decl);
return false;
}
Index: gcc/cp/cp-tree.h
===================================================================
--- gcc/cp/cp-tree.h 2020-12-31 16:51:34.000000000 +0000
+++ gcc/cp/cp-tree.h 2021-01-01 01:33:03.194982000 +0000
@@ -6435,5 +6435,5 @@ class access_failure_info
};
-extern void complain_about_access (tree, tree, bool);
+extern void complain_about_access (tree, tree, tree, bool, access_kind);
extern void push_defarg_context (tree);
extern void pop_defarg_context (void);
Index: gcc/cp/search.c
===================================================================
--- gcc/cp/search.c 2020-12-31 16:51:34.000000000 +0000
+++ gcc/cp/search.c 2021-01-01 01:33:56.788180000 +0000
@@ -48,5 +48,5 @@ static tree dfs_walk_once_accessible (tr
void *data);
static tree dfs_access_in_type (tree, void *);
-static access_kind access_in_type (tree, tree);
+access_kind access_in_type (tree, tree);
static tree dfs_get_pure_virtuals (tree, void *);
@@ -584,5 +584,5 @@ dfs_access_in_type (tree binfo, void *da
/* Return the access to DECL in TYPE. */
-static access_kind
+access_kind
access_in_type (tree type, tree decl)
{
Index: gcc/cp/typeck.c
===================================================================
--- gcc/cp/typeck.c 2020-12-31 16:51:34.000000000 +0000
+++ gcc/cp/typeck.c 2021-01-01 01:37:13.602815000 +0000
@@ -2981,5 +2981,6 @@ complain_about_unrecognized_member (tree
? TREE_TYPE (access_path) : object_type,
name, afi.get_diag_decl ());
- complain_about_access (afi.get_decl (), afi.get_diag_decl (), false);
+ complain_about_access (afi.get_decl (), afi.get_diag_decl (),
+ afi.get_diag_decl (), false, ak_none);
}
}
Index: gcc/cp/call.c
===================================================================
--- gcc/cp/call.c 2020-12-31 16:51:34.000000000 +0000
+++ gcc/cp/call.c 2021-01-04 19:40:18.564108581 +0000
@@ -7133,31 +7133,49 @@ build_op_delete_call (enum tree_code cod
in the diagnostics.
- If ISSUE_ERROR is true, then issue an error about the
- access, followed by a note showing the declaration.
- Otherwise, just show the note. */
+ If ISSUE_ERROR is true, then issue an error about the access, followed
+ by a note showing the declaration. Otherwise, just show the note.
+
+ DIAG_DECL and DIAG_LOCATION will almost always be the same.
+ DIAG_LOCATION is just another DECL. NO_ACCESS_REASON is an optional
+ parameter used to specify why DECL wasn't accessible (e.g. ak_private
+ would be because DECL was private). If not using NO_ACCESS_REASON,
+ then it must be ak_none, and the access failure reason will be
+ figured out by looking at the protection of DECL. */
void
-complain_about_access (tree decl, tree diag_decl, bool issue_error)
+complain_about_access (tree decl, tree diag_decl, tree diag_location,
+bool issue_error, access_kind no_access_reason)
{
- if (TREE_PRIVATE (decl))
- {
- if (issue_error)
- error ("%q#D is private within this context", diag_decl);
- inform (DECL_SOURCE_LOCATION (diag_decl),
- "declared private here");
- }
- else if (TREE_PROTECTED (decl))
- {
- if (issue_error)
- error ("%q#D is protected within this context", diag_decl);
- inform (DECL_SOURCE_LOCATION (diag_decl),
- "declared protected here");
- }
+ /* If we have not already figured out why DECL is innaccessible... */
+ if (no_access_reason == ak_none)
+ {
+ /* Examine the access of DECL to find out why. */
+ if (TREE_PRIVATE (decl))
+ no_access_reason = ak_private;
+ else if (TREE_PROTECTED (decl))
+ no_access_reason = ak_protected;
+ }
+
+ /* Now generate an error message depending on calculated access. */
+ if (no_access_reason == ak_private)
+ {
+ if (issue_error)
+ error ("%q#D is private within this context", diag_decl);
+ inform (DECL_SOURCE_LOCATION (diag_location), "declared private here");
+ }
+ else if (no_access_reason == ak_protected)
+ {
+ if (issue_error)
+ error ("%q#D is protected within this context", diag_decl);
+ inform (DECL_SOURCE_LOCATION (diag_location), "declared protected here");
+ }
+ /* Couldn't figure out why DECL is innaccesible, so just say it's
+ innaccessible. */
else
- {
- if (issue_error)
- error ("%q#D is inaccessible within this context", diag_decl);
- inform (DECL_SOURCE_LOCATION (diag_decl), "declared here");
- }
+ {
+ if (issue_error)
+ error ("%q#D is inaccessible within this context", diag_decl);
+ inform (DECL_SOURCE_LOCATION (diag_decl), "declared here");
+ }
}
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] c++: private inheritance access diagnostics fix [PR17314]
2021-01-05 14:24 [PATCH] c++: private inheritance access diagnostics fix [PR17314] Anthony Sharp
@ 2021-01-07 21:01 ` Jason Merrill
2021-01-09 0:38 ` Anthony Sharp
0 siblings, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2021-01-07 21:01 UTC (permalink / raw)
To: Anthony Sharp, gcc-patches
On 1/5/21 9:24 AM, Anthony Sharp via Gcc-patches wrote:
> This patch fixes PR17314 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=17314).
> 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 "private". This patch updates the
> diagnostics code to generate more correct errors in cases of failed
> inheritance such as these.
>
> The reason this bug happened was because GCC was examining the
> declared access of decl, instead of looking at it in the context of class
> inheritance.
>
> ----------- COMMENTS -----------
>
> This is my first GCC patch ever
Thanks, and welcome!
To start with, do you have a copyright assignment on file or in the
works already? If not, see the top of
https://gcc.gnu.org/contribute.html
for more information. I probably shouldn't look at the patch in detail
until that's addressed.
Second, your patch was mangled by word wrap so that it can't be applied
without manual repair. If you can't prevent word wrap in your mail
client, please send it as an attachment rather than inline.
Also, there are a few whitespace issues in the patch; please run
contrib/check_GNU_style.sh on the patch before submitting.
> so there is probably something I have done
> very wrong. Please let me know :) The thought of my code being scrutinised
> by people with PhDs and doctorates is quite frankly terrifying.
>
> Note that since it is a new year I had to make a new changelog file so the
> diff for the patch might be slightly off.
If you use contrib/gcc-git-customization.sh and then git
gcc-commit-mklog you don't need to touch ChangeLog files at all, just
adjust the generated ChangeLog entries in the git commit message. I
personally tend to commit first with a placeholder message and then use
git gcc-commit-mklog --amend to generate the ChangeLog entries.
> There was no need to add additional regression tests since it was adequate
> to simply change some of the regression tests that were there originally
> (all the patch changes is the informative message telling the user where
> a decl was defined as private).
Agreed.
> ----------- REGRESSION ANALYSIS -----------
>
> No regressions reported.
>
> G++ (CLEAN) RESULTS
>
> # of expected passes 202879
> # of unexpected failures 1
> # of expected failures 988
> # of unsupported tests 8654
>
> GCC (CLEAN) RESULTS
>
> # of expected passes 163377
> # of unexpected failures 94
> # of unexpected successes 37
> # of expected failures 915
> # of unsupported tests 2530
>
> G++ (PR17314 PATCHED) RESULTS
>
> # of expected passes 202871
> # of unexpected failures 1
> # of expected failures 988
> # of unsupported tests 8654
>
> GCC (PR17314 PATCHED) RESULTS
>
> # of expected passes 163377
> # of unexpected failures 94
> # of unexpected successes 37
> # of expected failures 915
> # of unsupported tests 2530
>
> When I build and make -k check -j 6 on the patched source it reports
> 202871 passes (8 fewer), although the FAILs do not increase. I am not 100%
> sure why this happens since I have not removed any testcases, only edited a
> few, but I think this happens because in files like dr142.c I removed more
> output checks than I added. make -k check -j 6 also returns error 2
> sometimes, although there are no obvious errors or warnings in the logs
> explaining why. Probably harmless?
Probably. Can you use sort/uniq/diff on the .sum testsuite output to
determine which passes are missing in the patched sources?
> ----------- BUILD REPORT -----------
>
> GCC builds normally on x86_64-pc-linux-gnu for x86_64-pc-linux-gnu using
> make -j 6. I didn't see it necessary to test on other build targets since the
> patch only affects the C++ front end and so functionality is unlikely
> to differ between platforms.
>
> The compile log reports:
>
> Comparing stages 2 and 3
> warning: gcc/cc1obj-checksum.o differs
> Comparison successful.
>
> and then continues. I assume this means it was actually successful.
Yes.
Thanks,
Jason
> Index: gcc/cp/ChangeLog
> from Anthony Sharp <anthonysharp15@gmail.com>
>
> Fixes PR17314
> * typeck.c (complain_about_unrecognized_member): Updated function
> arguments in complain_about_access.
> * call.c (complain_about_access): Altered function.
> * semantics.c (get_parent_with_private_access): Added function.
> (access_in_type): Added as extern function.
> * search.c (access_in_type): Made function non-static so it can be
> used in semantics.c.
> * cp-tree.h (complain_about_access): Changed parameters of function.
> Index: gcc/testsuite/ChangeLog
> from Anthony Sharp <anthonysharp15@gmail.com>
>
> Fixes PR17314
> * g++.dg/lookup/scoped1.c modified testcase to run successfully with
> changes.
> * g++.dg/tc1/dr142.c modified testcase to run successfully with
> changes.
> * g++.dg/tc1/dr142.c modified testcase to run successfully with
> changes.
> * g++.dg/tc1/dr142.c modified testcase to run successfully with
> changes.
> * g++.dg/tc1/dr52.c modified testcase to run successfully with changes.
> * g++.old-deja/g++.brendan/visibility6.c modified testcase to run
> successfully with changes.
> * g++.old-deja/g++.brendan/visibility8.c modified testcase to run
> successfully with changes.
> * g++.old-deja/g++.jason/access8.c modified testcase to run
> successfully with changes.
> * g++.old-deja/g++.law/access4.c modified testcase to run successfully
> with changes.
> * g++.old-deja/g++.law/visibility12.c modified testcase to run
> successfully with changes.
> * g++.old-deja/g++.law/visibility4.c modified testcase to run
> successfully with changes.
> * g++.old-deja/g++.law/visibility8.c modified testcase to run
> successfully with changes.
> * g++.old-deja/g++.other/access4.c modified testcase to run
> successfully with changes.
> Index: gcc/testsuite/g++.old-deja/g++.jason/access8.C
> ===================================================================
> --- gcc/testsuite/g++.old-deja/g++.jason/access8.C 2020-12-31
> 16:51:34.000000000 +0000
> +++ gcc/testsuite/g++.old-deja/g++.jason/access8.C 2021-01-03
> 00:22:14.969854000 +0000
> @@ -4,5 +4,5 @@
> // Bug: g++ forgets access decls after the definition.
>
> -class inh { // { dg-message "" } inaccessible
> +class inh {
> int a;
> protected:
> @@ -10,5 +10,6 @@ protected:
> };
>
> -class mel : private inh {
> +class mel : private inh // { dg-message "" } inaccessible
> +{
> protected:
> int t;
> Index: gcc/testsuite/g++.old-deja/g++.law/access4.C
> ===================================================================
> --- gcc/testsuite/g++.old-deja/g++.law/access4.C 2020-12-31
> 16:51:34.000000000 +0000
> +++ gcc/testsuite/g++.old-deja/g++.law/access4.C 2021-01-03
> 00:21:01.736320000 +0000
> @@ -7,10 +7,11 @@
> // Message-ID: <9403030024.AA04534@ses.com>
>
> -class ForceLeafSterile { // { dg-message "" }
> +class ForceLeafSterile {
> friend class Sterile;
> ForceLeafSterile() {} // { dg-message "" }
> };
>
> -class Sterile : private virtual ForceLeafSterile {
> +class Sterile : private virtual ForceLeafSterile // { dg-message "" }
> +{
> public:
> Sterile() {}
> Index: gcc/testsuite/g++.old-deja/g++.law/visibility12.C
> ===================================================================
> --- gcc/testsuite/g++.old-deja/g++.law/visibility12.C 2020-12-31
> 16:51:34.000000000 +0000
> +++ gcc/testsuite/g++.old-deja/g++.law/visibility12.C 2021-01-03
> 00:20:24.243535000 +0000
> @@ -7,9 +7,10 @@
> // Message-ID: <9306300528.AA17185@coda.mel.dit.CSIRO.AU>
> struct a {
> - int aa; // { dg-message "" } private
> + int aa;
> };
>
> -class b : private a {
> - };
> +class b : private a // { dg-message "" } private
> +{
> +};
>
> class c : public b {
> Index: gcc/testsuite/g++.old-deja/g++.law/visibility8.C
> ===================================================================
> --- gcc/testsuite/g++.old-deja/g++.law/visibility8.C 2020-12-31
> 16:51:34.000000000 +0000
> +++ gcc/testsuite/g++.old-deja/g++.law/visibility8.C 2021-01-03
> 00:19:45.574725000 +0000
> @@ -8,9 +8,10 @@
> class t1 {
> protected:
> - int a; // { dg-message "" } protected
> + int a;
> };
>
>
> -class t2 : private t1 {
> +class t2 : private t1 // { dg-message "" } protected
> +{
> public:
> int b;
> Index: gcc/testsuite/g++.old-deja/g++.law/visibility4.C
> ===================================================================
> --- gcc/testsuite/g++.old-deja/g++.law/visibility4.C 2020-12-31
> 16:51:34.000000000 +0000
> +++ gcc/testsuite/g++.old-deja/g++.law/visibility4.C 2021-01-03
> 00:20:06.815170000 +0000
> @@ -9,8 +9,9 @@
> class A {
> public:
> - int b; // { dg-message "" } private
> + int b;
> };
>
> -class C : private A { // NOTE WELL. private, not public
> +class C : private A // { dg-message "" } private
> +{
> public:
> int d;
> Index: gcc/testsuite/g++.old-deja/g++.other/access4.C
> ===================================================================
> --- gcc/testsuite/g++.old-deja/g++.other/access4.C 2020-12-31
> 16:51:34.000000000 +0000
> +++ gcc/testsuite/g++.old-deja/g++.other/access4.C 2021-01-03
> 00:19:25.362302000 +0000
> @@ -1,9 +1,9 @@
> // { dg-do assemble }
>
> -struct A { // { dg-message "" } inaccessible
> +struct A {
> static int i;
> };
>
> -struct B : private A { };
> +struct B : private A { }; // { dg-message "" } inaccessible
>
> struct C : public B {
> Index: gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C
> ===================================================================
> --- gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C 2020-12-31
> 16:51:34.000000000 +0000
> +++ gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C 2021-01-03
> 00:19:17.918146000 +0000
> @@ -6,7 +6,7 @@ class foo
> {
> public:
> - static int y; // { dg-message "" } private
> + static int y;
> };
> -class foo1 : private foo
> +class foo1 : private foo // { dg-message "" } private
> { };
> class foo2 : public foo1
> Index: gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C
> ===================================================================
> --- gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C 2020-12-31
> 16:51:34.000000000 +0000
> +++ gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C 2021-01-03
> 00:21:55.873454000 +0000
> @@ -4,7 +4,7 @@ class bottom
> {
> public:
> - int b; // { dg-message "" } private
> + int b;
> };
> -class middle : private bottom
> +class middle : private bottom // { dg-message "" } private
> {
> public:
> Index: gcc/testsuite/g++.dg/lookup/scoped1.C
> ===================================================================
> --- gcc/testsuite/g++.dg/lookup/scoped1.C 2020-12-31 16:51:34.000000000 +0000
> +++ gcc/testsuite/g++.dg/lookup/scoped1.C 2021-01-02 21:51:30.088674000 +0000
> @@ -5,10 +5,10 @@ struct A
> {
> static int i1;
> - int i2; // { dg-message "declared" }
> + int i2;
> static void f1 ();
> void f2 ();
> };
>
> -struct B: private A { };
> +struct B: private A { }; // { dg-message "declared" }
> struct C: public B
> {
> Index: gcc/testsuite/g++.dg/tc1/dr142.C
> ===================================================================
> --- gcc/testsuite/g++.dg/tc1/dr142.C 2020-12-31 16:51:34.000000000 +0000
> +++ gcc/testsuite/g++.dg/tc1/dr142.C 2021-01-02 21:54:09.839546000 +0000
> @@ -3,11 +3,11 @@
> // DR142: Injection-related errors in access example
>
> -class B { // { dg-message "declared" }
> +class B {
> public:
> - int mi; // { dg-message "declared" }
> - static int si; // { dg-message "declared" }
> + int mi;
> + static int si;
> };
>
> -class D: private B {
> +class D: private B { // { dg-message "declared" }
> };
>
> Index: gcc/testsuite/g++.dg/tc1/dr52.C
> ===================================================================
> --- gcc/testsuite/g++.dg/tc1/dr52.C 2020-12-31 16:51:34.000000000 +0000
> +++ gcc/testsuite/g++.dg/tc1/dr52.C 2021-01-03 13:20:08.267946000 +0000
> @@ -18,9 +18,9 @@ struct B2 : B {};
>
> struct C
> -{ // { dg-message "declared" }
> - void foo(void);
> +{
> + void foo(void);
> };
>
> -struct D : private C {};
> +struct D : private C {}; // { dg-message "declared" }
>
> struct X: A, B1, B2, D
> Index: gcc/cp/semantics.c
> ===================================================================
> --- gcc/cp/semantics.c 2020-12-31 16:51:34.000000000 +0000
> +++ gcc/cp/semantics.c 2021-01-04 20:26:51.912302840 +0000
> @@ -47,4 +47,6 @@ along with GCC; see the file COPYING3.
> #include "memmodel.h"
>
> +extern access_kind access_in_type (tree type, tree decl);
> +
> /* There routines provide a modular interface to perform many parsing
> operations. They may therefore be used during actual parsing, or
> @@ -257,4 +259,39 @@ pop_to_parent_deferring_access_checks (v
> }
>
> +/* This deals with bug PR17314.
> +
> + DECL is a declaration and BINFO represents a class that has attempted (but
> + failed) to access DECL.
> +
> + Examine the parent binfos of BINFO and determine whether any of them had
> + private access to DECL. If they did, return the parent binfo. This helps
> + in figuring out the correct error message to show (if the parents had
> + access, it's their fault for not giving sufficient access to BINFO).
> +
> + If no parents had access, return NULL_TREE. */
> +
> +static tree
> +get_parent_with_private_access(tree decl, tree binfo)
> +{
> + /* Only BINFOs should come through here. */
> + gcc_assert(TREE_CODE(binfo) == TREE_BINFO);
> +
> + tree base_binfo = NULL_TREE;
> +
> + /* Iterate through immediate parent classes. */
> + for (int i = 0; BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
> + {
> + /* This parent had private access. Therefore that's why BINFO can't access
> + DECL. */
> + if(access_in_type(BINFO_TYPE(base_binfo), decl) == ak_private)
> + return base_binfo;
> + }
> +
> + /* None of the parents had access. Note: it's impossible for one of the
> + parents to have had public or protected access to DECL, since then
> + BINFO would have been able to access DECL too. */
> + return NULL_TREE;
> +}
> +
> /* If the current scope isn't allowed to access DECL along
> BASETYPE_PATH, give an error, or if we're parsing a function or class
> @@ -315,9 +352,34 @@ enforce_access (tree basetype_path, tree
> {
> if (flag_new_inheriting_ctors)
> - diag_decl = strip_inheriting_ctors (diag_decl);
> + diag_decl = strip_inheriting_ctors (diag_decl);
> if (complain & tf_error)
> - complain_about_access (decl, diag_decl, true);
> + {
> + /* We will usually want to point to the same place as diag_decl but
> + not always. */
> + tree diag_location = diag_decl;
> + access_kind parent_access = ak_none;
> +
> + /* See if any of BASETYPE_PATH's parents had private access to DECL.
> + If they did, that will tell us why we don't. */
> + tree parent_binfo = get_parent_with_private_access(decl,
> basetype_path);
> +
> + /* If a parent had private access, then the diagnostic location DECL
> + should be that of the parent class, since it failed to give suitable
> + access by using a private inheritance. But if DECL was actually defined
> + in the parent, it wasn't privately inherited, and so we don't need to
> + do this, and complain_about_access will figure out what to do. */
> + if(parent_binfo != NULL_TREE
> + && context_for_name_lookup(decl) != BINFO_TYPE(parent_binfo))
> + {
> + diag_location = TYPE_NAME(BINFO_TYPE(parent_binfo));
> + parent_access = ak_private;
> + }
> +
> + /* Finally, generate an error message. */
> + complain_about_access (decl, diag_decl, diag_location, true,
> + parent_access);
> + }
> if (afi)
> - afi->record_access_failure (basetype_path, decl, diag_decl);
> + afi->record_access_failure (basetype_path, decl, diag_decl);
> return false;
> }
> Index: gcc/cp/cp-tree.h
> ===================================================================
> --- gcc/cp/cp-tree.h 2020-12-31 16:51:34.000000000 +0000
> +++ gcc/cp/cp-tree.h 2021-01-01 01:33:03.194982000 +0000
> @@ -6435,5 +6435,5 @@ class access_failure_info
> };
>
> -extern void complain_about_access (tree, tree, bool);
> +extern void complain_about_access (tree, tree, tree, bool, access_kind);
> extern void push_defarg_context (tree);
> extern void pop_defarg_context (void);
> Index: gcc/cp/search.c
> ===================================================================
> --- gcc/cp/search.c 2020-12-31 16:51:34.000000000 +0000
> +++ gcc/cp/search.c 2021-01-01 01:33:56.788180000 +0000
> @@ -48,5 +48,5 @@ static tree dfs_walk_once_accessible (tr
> void *data);
> static tree dfs_access_in_type (tree, void *);
> -static access_kind access_in_type (tree, tree);
> +access_kind access_in_type (tree, tree);
> static tree dfs_get_pure_virtuals (tree, void *);
>
> @@ -584,5 +584,5 @@ dfs_access_in_type (tree binfo, void *da
> /* Return the access to DECL in TYPE. */
>
> -static access_kind
> +access_kind
> access_in_type (tree type, tree decl)
> {
> Index: gcc/cp/typeck.c
> ===================================================================
> --- gcc/cp/typeck.c 2020-12-31 16:51:34.000000000 +0000
> +++ gcc/cp/typeck.c 2021-01-01 01:37:13.602815000 +0000
> @@ -2981,5 +2981,6 @@ complain_about_unrecognized_member (tree
> ? TREE_TYPE (access_path) : object_type,
> name, afi.get_diag_decl ());
> - complain_about_access (afi.get_decl (), afi.get_diag_decl (), false);
> + complain_about_access (afi.get_decl (), afi.get_diag_decl (),
> + afi.get_diag_decl (), false, ak_none);
> }
> }
> Index: gcc/cp/call.c
> ===================================================================
> --- gcc/cp/call.c 2020-12-31 16:51:34.000000000 +0000
> +++ gcc/cp/call.c 2021-01-04 19:40:18.564108581 +0000
> @@ -7133,31 +7133,49 @@ build_op_delete_call (enum tree_code cod
> in the diagnostics.
>
> - If ISSUE_ERROR is true, then issue an error about the
> - access, followed by a note showing the declaration.
> - Otherwise, just show the note. */
> + If ISSUE_ERROR is true, then issue an error about the access, followed
> + by a note showing the declaration. Otherwise, just show the note.
> +
> + DIAG_DECL and DIAG_LOCATION will almost always be the same.
> + DIAG_LOCATION is just another DECL. NO_ACCESS_REASON is an optional
> + parameter used to specify why DECL wasn't accessible (e.g. ak_private
> + would be because DECL was private). If not using NO_ACCESS_REASON,
> + then it must be ak_none, and the access failure reason will be
> + figured out by looking at the protection of DECL. */
>
> void
> -complain_about_access (tree decl, tree diag_decl, bool issue_error)
> +complain_about_access (tree decl, tree diag_decl, tree diag_location,
> +bool issue_error, access_kind no_access_reason)
> {
> - if (TREE_PRIVATE (decl))
> - {
> - if (issue_error)
> - error ("%q#D is private within this context", diag_decl);
> - inform (DECL_SOURCE_LOCATION (diag_decl),
> - "declared private here");
> - }
> - else if (TREE_PROTECTED (decl))
> - {
> - if (issue_error)
> - error ("%q#D is protected within this context", diag_decl);
> - inform (DECL_SOURCE_LOCATION (diag_decl),
> - "declared protected here");
> - }
> + /* If we have not already figured out why DECL is innaccessible... */
> + if (no_access_reason == ak_none)
> + {
> + /* Examine the access of DECL to find out why. */
> + if (TREE_PRIVATE (decl))
> + no_access_reason = ak_private;
> + else if (TREE_PROTECTED (decl))
> + no_access_reason = ak_protected;
> + }
> +
> + /* Now generate an error message depending on calculated access. */
> + if (no_access_reason == ak_private)
> + {
> + if (issue_error)
> + error ("%q#D is private within this context", diag_decl);
> + inform (DECL_SOURCE_LOCATION (diag_location), "declared private here");
> + }
> + else if (no_access_reason == ak_protected)
> + {
> + if (issue_error)
> + error ("%q#D is protected within this context", diag_decl);
> + inform (DECL_SOURCE_LOCATION (diag_location), "declared protected here");
> + }
> + /* Couldn't figure out why DECL is innaccesible, so just say it's
> + innaccessible. */
> else
> - {
> - if (issue_error)
> - error ("%q#D is inaccessible within this context", diag_decl);
> - inform (DECL_SOURCE_LOCATION (diag_decl), "declared here");
> - }
> + {
> + if (issue_error)
> + error ("%q#D is inaccessible within this context", diag_decl);
> + inform (DECL_SOURCE_LOCATION (diag_decl), "declared here");
> + }
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] c++: private inheritance access diagnostics fix [PR17314]
2021-01-07 21:01 ` Jason Merrill
@ 2021-01-09 0:38 ` Anthony Sharp
2021-01-11 20:03 ` Jason Merrill
0 siblings, 1 reply; 10+ messages in thread
From: Anthony Sharp @ 2021-01-09 0:38 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 4273 bytes --]
Hi Jason,
Thank you!
> To start with, do you have a copyright assignment on file or in the
> works already?
Good point. I incorrectly assumed it would only be a minor
contribution copyright-wise. Mr Edelsohn gave me a template which I've
now filled out and sent to assign@gnu.org. I'm assuming I just need to
wait for them to send me the form. I'll update this thread when that's
sorted. In the meantime I've hopefully fixed some of the issues.
> Second, your patch was mangled by word wrap so that it can't be applied
> without manual repair. If you can't prevent word wrap in your mail
> client, please send it as an attachment rather than inline.
Oh yes I see where it's gotten mangled now. I'm attaching it as a
.patch file (I assume that's okay).
> Also, there are a few whitespace issues in the patch; please run
> contrib/check_GNU_style.sh on the patch before submitting.
Should be all fixed now (there is one style issue left but it's a
false positive). Visual Studio Code was lying to me about what the
file looks like so if there are any more formatting issues please let
me know.
> If you use contrib/gcc-git-customization.sh and then git
> gcc-commit-mklog you don't need to touch ChangeLog files at all, just
> adjust the generated ChangeLog entries in the git commit message. I
> personally tend to commit first with a placeholder message and then use
> git gcc-commit-mklog --amend to generate the ChangeLog entries.
Wouldn't that require read-write access? (Just from looking here
https://gcc.gnu.org/gitwrite.html.)
> Probably. Can you use sort/uniq/diff on the .sum testsuite output to
> determine which passes are missing in the patched sources?
According to contrib/dg-cmp-results.sh ...
I get a bunch of these weird NA->PASSes (and vice-versa), for example:
PASS->NA: g++.dg/modules/alias-1_a.H module-cmi
(gcm.cache/home/anthony/Desktop/GCC/builds_and_source/source_clean/gcc/testsuite/g++.dg/modules/alias-1_a.H.gcm)
NA->PASS: g++.dg/modules/alias-1_a.H module-cmi
(gcm.cache/home/anthony/Desktop/GCC/builds_and_source/source_pr17314/gcc/testsuite/g++.dg/modules/alias-1_a.H.gcm)
PASS->NA: g++.dg/modules/alias-1_a.H module-cmi
(gcm.cache/home/anthony/Desktop/GCC/builds_and_source/source_clean/gcc/testsuite/g++.dg/modules/alias-1_a.H.gcm)
NA->PASS: g++.dg/modules/alias-1_a.H module-cmi
(gcm.cache/home/anthony/Desktop/GCC/builds_and_source/source_pr17314/gcc/testsuite/g++.dg/modules/alias-1_a.H.gcm)
They're weird because I haven't actually touched those files (so I'm
assuming this is normal). There are about ~400 of those and they're
all .gcm files. They seem to balance out.
dr142.c reports:
NA->PASS: g++.dg/tc1/dr142.C -std=c++14 (test for warnings, line 11)
PASS->NA: g++.dg/tc1/dr142.C -std=c++14 (test for warnings, line 5)
PASS->NA: g++.dg/tc1/dr142.C -std=c++14 (test for warnings, line 7)
PASS->NA: g++.dg/tc1/dr142.C -std=c++14 (test for warnings, line 8)
NA->PASS: g++.dg/tc1/dr142.C -std=c++17 (test for warnings, line 11)
PASS->NA: g++.dg/tc1/dr142.C -std=c++17 (test for warnings, line 5)
PASS->NA: g++.dg/tc1/dr142.C -std=c++17 (test for warnings, line 7)
PASS->NA: g++.dg/tc1/dr142.C -std=c++17 (test for warnings, line 8)
NA->PASS: g++.dg/tc1/dr142.C -std=c++2a (test for warnings, line 11)
PASS->NA: g++.dg/tc1/dr142.C -std=c++2a (test for warnings, line 5)
PASS->NA: g++.dg/tc1/dr142.C -std=c++2a (test for warnings, line 7)
PASS->NA: g++.dg/tc1/dr142.C -std=c++2a (test for warnings, line 8)
NA->PASS: g++.dg/tc1/dr142.C -std=c++98 (test for warnings, line 11)
PASS->NA: g++.dg/tc1/dr142.C -std=c++98 (test for warnings, line 5)
PASS->NA: g++.dg/tc1/dr142.C -std=c++98 (test for warnings, line 7)
PASS->NA: g++.dg/tc1/dr142.C -std=c++98 (test for warnings, line 8)
In other words, there are 12 PASS->NAs and 4 NA->PASSes in this file,
meaning a net change of -8 (which explains why there are eight fewer).
My other changes also report PASS->NAs and vice-versa, but for those
the number of new NAs equals the number of new PASSes, so they don't
cause a change in quantity.
Thanks for being patient with me. I'll let you know when I've
completed the forms.
Also if I need to adjust the .patch to deal with the changelogs issue
please let me know.
Kind regards,
Anthony
[-- Attachment #2: pr17314.patch --]
[-- Type: text/x-patch, Size: 15991 bytes --]
Index: gcc/testsuite/g++.old-deja/g++.jason/access8.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.jason/access8.C 2020-12-31 16:51:34.000000000 +0000
+++ gcc/testsuite/g++.old-deja/g++.jason/access8.C 2021-01-03 00:22:14.969854000 +0000
@@ -4,5 +4,5 @@
// Bug: g++ forgets access decls after the definition.
-class inh { // { dg-message "" } inaccessible
+class inh {
int a;
protected:
@@ -10,5 +10,6 @@ protected:
};
-class mel : private inh {
+class mel : private inh // { dg-message "" } inaccessible
+{
protected:
int t;
Index: gcc/testsuite/g++.old-deja/g++.law/access4.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.law/access4.C 2020-12-31 16:51:34.000000000 +0000
+++ gcc/testsuite/g++.old-deja/g++.law/access4.C 2021-01-03 00:21:01.736320000 +0000
@@ -7,10 +7,11 @@
// Message-ID: <9403030024.AA04534@ses.com>
-class ForceLeafSterile { // { dg-message "" }
+class ForceLeafSterile {
friend class Sterile;
ForceLeafSterile() {} // { dg-message "" }
};
-class Sterile : private virtual ForceLeafSterile {
+class Sterile : private virtual ForceLeafSterile // { dg-message "" }
+{
public:
Sterile() {}
Index: gcc/testsuite/g++.old-deja/g++.law/visibility12.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.law/visibility12.C 2020-12-31 16:51:34.000000000 +0000
+++ gcc/testsuite/g++.old-deja/g++.law/visibility12.C 2021-01-03 00:20:24.243535000 +0000
@@ -7,9 +7,10 @@
// Message-ID: <9306300528.AA17185@coda.mel.dit.CSIRO.AU>
struct a {
- int aa; // { dg-message "" } private
+ int aa;
};
-class b : private a {
- };
+class b : private a // { dg-message "" } private
+{
+};
class c : public b {
Index: gcc/testsuite/g++.old-deja/g++.law/visibility8.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.law/visibility8.C 2020-12-31 16:51:34.000000000 +0000
+++ gcc/testsuite/g++.old-deja/g++.law/visibility8.C 2021-01-03 00:19:45.574725000 +0000
@@ -8,9 +8,10 @@
class t1 {
protected:
- int a; // { dg-message "" } protected
+ int a;
};
-class t2 : private t1 {
+class t2 : private t1 // { dg-message "" } protected
+{
public:
int b;
Index: gcc/testsuite/g++.old-deja/g++.law/visibility4.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.law/visibility4.C 2020-12-31 16:51:34.000000000 +0000
+++ gcc/testsuite/g++.old-deja/g++.law/visibility4.C 2021-01-03 00:20:06.815170000 +0000
@@ -9,8 +9,9 @@
class A {
public:
- int b; // { dg-message "" } private
+ int b;
};
-class C : private A { // NOTE WELL. private, not public
+class C : private A // { dg-message "" } private
+{
public:
int d;
Index: gcc/testsuite/g++.old-deja/g++.other/access4.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.other/access4.C 2020-12-31 16:51:34.000000000 +0000
+++ gcc/testsuite/g++.old-deja/g++.other/access4.C 2021-01-03 00:19:25.362302000 +0000
@@ -1,9 +1,9 @@
// { dg-do assemble }
-struct A { // { dg-message "" } inaccessible
+struct A {
static int i;
};
-struct B : private A { };
+struct B : private A { }; // { dg-message "" } inaccessible
struct C : public B {
Index: gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C 2020-12-31 16:51:34.000000000 +0000
+++ gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C 2021-01-03 00:19:17.918146000 +0000
@@ -6,7 +6,7 @@ class foo
{
public:
- static int y; // { dg-message "" } private
+ static int y;
};
-class foo1 : private foo
+class foo1 : private foo // { dg-message "" } private
{ };
class foo2 : public foo1
Index: gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C 2020-12-31 16:51:34.000000000 +0000
+++ gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C 2021-01-03 00:21:55.873454000 +0000
@@ -4,7 +4,7 @@ class bottom
{
public:
- int b; // { dg-message "" } private
+ int b;
};
-class middle : private bottom
+class middle : private bottom // { dg-message "" } private
{
public:
Index: gcc/testsuite/g++.dg/lookup/scoped1.C
===================================================================
--- gcc/testsuite/g++.dg/lookup/scoped1.C 2020-12-31 16:51:34.000000000 +0000
+++ gcc/testsuite/g++.dg/lookup/scoped1.C 2021-01-02 21:51:30.088674000 +0000
@@ -5,10 +5,10 @@ struct A
{
static int i1;
- int i2; // { dg-message "declared" }
+ int i2;
static void f1 ();
void f2 ();
};
-struct B: private A { };
+struct B: private A { }; // { dg-message "declared" }
struct C: public B
{
Index: gcc/testsuite/g++.dg/tc1/dr142.C
===================================================================
--- gcc/testsuite/g++.dg/tc1/dr142.C 2020-12-31 16:51:34.000000000 +0000
+++ gcc/testsuite/g++.dg/tc1/dr142.C 2021-01-02 21:54:09.839546000 +0000
@@ -3,11 +3,11 @@
// DR142: Injection-related errors in access example
-class B { // { dg-message "declared" }
+class B {
public:
- int mi; // { dg-message "declared" }
- static int si; // { dg-message "declared" }
+ int mi;
+ static int si;
};
-class D: private B {
+class D: private B { // { dg-message "declared" }
};
Index: gcc/testsuite/g++.dg/tc1/dr52.C
===================================================================
--- gcc/testsuite/g++.dg/tc1/dr52.C 2020-12-31 16:51:34.000000000 +0000
+++ gcc/testsuite/g++.dg/tc1/dr52.C 2021-01-03 13:20:08.267946000 +0000
@@ -18,9 +18,9 @@ struct B2 : B {};
struct C
-{ // { dg-message "declared" }
- void foo(void);
+{
+ void foo(void);
};
-struct D : private C {};
+struct D : private C {}; // { dg-message "declared" }
struct X: A, B1, B2, D
Index: gcc/testsuite/ChangeLog
from Anthony Sharp <anthonysharp15@gmail.com>
Fixes PR17314
* g++.dg/lookup/scoped1.c modified testcase to run successfully with
changes.
* g++.dg/tc1/dr142.c modified testcase to run successfully with
changes.
* g++.dg/tc1/dr142.c modified testcase to run successfully with
changes.
* g++.dg/tc1/dr142.c modified testcase to run successfully with
changes.
* g++.dg/tc1/dr52.c modified testcase to run successfully with changes.
* g++.old-deja/g++.brendan/visibility6.c modified testcase to run
successfully with changes.
* g++.old-deja/g++.brendan/visibility8.c modified testcase to run
successfully with changes.
* g++.old-deja/g++.jason/access8.c modified testcase to run
successfully with changes.
* g++.old-deja/g++.law/access4.c modified testcase to run successfully
with changes.
* g++.old-deja/g++.law/visibility12.c modified testcase to run
successfully with changes.
* g++.old-deja/g++.law/visibility4.c modified testcase to run
successfully with changes.
* g++.old-deja/g++.law/visibility8.c modified testcase to run
successfully with changes.
* g++.old-deja/g++.other/access4.c modified testcase to run
successfully with changes.
Index: gcc/cp/semantics.c
===================================================================
--- gcc/cp/semantics.c 2020-12-31 16:51:34.000000000 +0000
+++ gcc/cp/semantics.c 2021-01-09 00:03:36.308446274 +0000
@@ -47,4 +47,6 @@ along with GCC; see the file COPYING3.
#include "memmodel.h"
+extern access_kind access_in_type (tree type, tree decl);
+
/* There routines provide a modular interface to perform many parsing
operations. They may therefore be used during actual parsing, or
@@ -257,4 +259,39 @@ pop_to_parent_deferring_access_checks (v
}
+/* This deals with bug PR17314.
+
+ DECL is a declaration and BINFO represents a class that has attempted (but
+ failed) to access DECL.
+
+ Examine the parent binfos of BINFO and determine whether any of them had
+ private access to DECL. If they did, return the parent binfo. This helps
+ in figuring out the correct error message to show (if the parents had
+ access, it's their fault for not giving sufficient access to BINFO).
+
+ If no parents had access, return NULL_TREE. */
+
+static tree
+get_parent_with_private_access (tree decl, tree binfo)
+{
+ /* Only BINFOs should come through here. */
+ gcc_assert (TREE_CODE (binfo) == TREE_BINFO);
+
+ tree base_binfo = NULL_TREE;
+
+ /* Iterate through immediate parent classes. */
+ for (int i = 0; BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
+ {
+ /* This parent had private access. Therefore that's why BINFO can't
+ access DECL. */
+ if (access_in_type (BINFO_TYPE (base_binfo), decl) == ak_private)
+ return base_binfo;
+ }
+
+ /* None of the parents had access. Note: it's impossible for one of the
+ parents to have had public or protected access to DECL, since then
+ BINFO would have been able to access DECL too. */
+ return NULL_TREE;
+}
+
/* If the current scope isn't allowed to access DECL along
BASETYPE_PATH, give an error, or if we're parsing a function or class
@@ -315,9 +352,38 @@ enforce_access (tree basetype_path, tree
{
if (flag_new_inheriting_ctors)
- diag_decl = strip_inheriting_ctors (diag_decl);
+ diag_decl = strip_inheriting_ctors (diag_decl);
if (complain & tf_error)
- complain_about_access (decl, diag_decl, true);
+ {
+ /* We will usually want to point to the same place as
+ diag_decl but not always. */
+ tree diag_location = diag_decl;
+ access_kind parent_access = ak_none;
+
+ /* See if any of BASETYPE_PATH's parents had private access
+ to DECL. If they did, that will tell us why we don't. */
+ tree parent_binfo = get_parent_with_private_access (decl,
+ basetype_path);
+
+ /* If a parent had private access, then the diagnostic
+ location DECL should be that of the parent class, since it
+ failed to give suitable access by using a private
+ inheritance. But if DECL was actually defined in the parent,
+ it wasn't privately inherited, and so we don't need to do
+ this, and complain_about_access will figure out what to
+ do. */
+ if (parent_binfo != NULL_TREE
+ && context_for_name_lookup (decl)
+ != BINFO_TYPE (parent_binfo))
+ {
+ diag_location = TYPE_NAME (BINFO_TYPE (parent_binfo));
+ parent_access = ak_private;
+ }
+
+ /* Finally, generate an error message. */
+ complain_about_access (decl, diag_decl, diag_location, true,
+ parent_access);
+ }
if (afi)
- afi->record_access_failure (basetype_path, decl, diag_decl);
+ afi->record_access_failure (basetype_path, decl, diag_decl);
return false;
}
Index: gcc/cp/cp-tree.h
===================================================================
--- gcc/cp/cp-tree.h 2020-12-31 16:51:34.000000000 +0000
+++ gcc/cp/cp-tree.h 2021-01-08 22:45:02.106467958 +0000
@@ -6435,5 +6435,6 @@ class access_failure_info
};
-extern void complain_about_access (tree, tree, bool);
+extern void complain_about_access (tree, tree, tree, bool,
+ access_kind);
extern void push_defarg_context (tree);
extern void pop_defarg_context (void);
Index: gcc/cp/search.c
===================================================================
--- gcc/cp/search.c 2020-12-31 16:51:34.000000000 +0000
+++ gcc/cp/search.c 2021-01-01 01:33:56.788180000 +0000
@@ -48,5 +48,5 @@ static tree dfs_walk_once_accessible (tr
void *data);
static tree dfs_access_in_type (tree, void *);
-static access_kind access_in_type (tree, tree);
+access_kind access_in_type (tree, tree);
static tree dfs_get_pure_virtuals (tree, void *);
@@ -584,5 +584,5 @@ dfs_access_in_type (tree binfo, void *da
/* Return the access to DECL in TYPE. */
-static access_kind
+access_kind
access_in_type (tree type, tree decl)
{
Index: gcc/cp/typeck.c
===================================================================
--- gcc/cp/typeck.c 2020-12-31 16:51:34.000000000 +0000
+++ gcc/cp/typeck.c 2021-01-01 01:37:13.602815000 +0000
@@ -2981,5 +2981,6 @@ complain_about_unrecognized_member (tree
? TREE_TYPE (access_path) : object_type,
name, afi.get_diag_decl ());
- complain_about_access (afi.get_decl (), afi.get_diag_decl (), false);
+ complain_about_access (afi.get_decl (), afi.get_diag_decl (),
+ afi.get_diag_decl (), false, ak_none);
}
}
Index: gcc/cp/call.c
===================================================================
--- gcc/cp/call.c 2020-12-31 16:51:34.000000000 +0000
+++ gcc/cp/call.c 2021-01-09 00:08:05.249597968 +0000
@@ -7133,31 +7133,49 @@ build_op_delete_call (enum tree_code cod
in the diagnostics.
- If ISSUE_ERROR is true, then issue an error about the
- access, followed by a note showing the declaration.
- Otherwise, just show the note. */
+ If ISSUE_ERROR is true, then issue an error about the access, followed
+ by a note showing the declaration. Otherwise, just show the note.
+
+ DIAG_DECL and DIAG_LOCATION will almost always be the same.
+ DIAG_LOCATION is just another DECL. NO_ACCESS_REASON is an optional
+ parameter used to specify why DECL wasn't accessible (e.g. ak_private
+ would be because DECL was private). If not using NO_ACCESS_REASON,
+ then it must be ak_none, and the access failure reason will be
+ figured out by looking at the protection of DECL. */
void
-complain_about_access (tree decl, tree diag_decl, bool issue_error)
+complain_about_access (tree decl, tree diag_decl, tree diag_location,
+bool issue_error, access_kind no_access_reason)
{
- if (TREE_PRIVATE (decl))
- {
- if (issue_error)
- error ("%q#D is private within this context", diag_decl);
- inform (DECL_SOURCE_LOCATION (diag_decl),
- "declared private here");
- }
- else if (TREE_PROTECTED (decl))
- {
- if (issue_error)
- error ("%q#D is protected within this context", diag_decl);
- inform (DECL_SOURCE_LOCATION (diag_decl),
- "declared protected here");
- }
+ /* If we have not already figured out why DECL is innaccessible... */
+ if (no_access_reason == ak_none)
+ {
+ /* Examine the access of DECL to find out why. */
+ if (TREE_PRIVATE (decl))
+ no_access_reason = ak_private;
+ else if (TREE_PROTECTED (decl))
+ no_access_reason = ak_protected;
+ }
+
+ /* Now generate an error message depending on calculated access. */
+ if (no_access_reason == ak_private)
+ {
+ if (issue_error)
+ error ("%q#D is private within this context", diag_decl);
+ inform (DECL_SOURCE_LOCATION (diag_location), "declared private here");
+ }
+ else if (no_access_reason == ak_protected)
+ {
+ if (issue_error)
+ error ("%q#D is protected within this context", diag_decl);
+ inform (DECL_SOURCE_LOCATION (diag_location), "declared protected here");
+ }
+ /* Couldn't figure out why DECL is innaccesible, so just say it's
+ innaccessible. */
else
- {
- if (issue_error)
- error ("%q#D is inaccessible within this context", diag_decl);
- inform (DECL_SOURCE_LOCATION (diag_decl), "declared here");
- }
+ {
+ if (issue_error)
+ error ("%q#D is inaccessible within this context", diag_decl);
+ inform (DECL_SOURCE_LOCATION (diag_decl), "declared here");
+ }
}
Index: gcc/cp/ChangeLog
from Anthony Sharp <anthonysharp15@gmail.com>
Fixes PR17314
* typeck.c (complain_about_unrecognized_member): Updated function
arguments in complain_about_access.
* call.c (complain_about_access): Altered function.
* semantics.c (get_parent_with_private_access): Added function.
(access_in_type): Added as extern function.
* search.c (access_in_type): Made function non-static so it can be
used in semantics.c.
* cp-tree.h (complain_about_access): Changed parameters of function.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] c++: private inheritance access diagnostics fix [PR17314]
2021-01-09 0:38 ` Anthony Sharp
@ 2021-01-11 20:03 ` Jason Merrill
2021-01-21 19:28 ` Anthony Sharp
0 siblings, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2021-01-11 20:03 UTC (permalink / raw)
To: Anthony Sharp; +Cc: gcc-patches
On 1/8/21 7:38 PM, Anthony Sharp wrote:
> Hi Jason,
>
> Thank you!
>
>> To start with, do you have a copyright assignment on file or in the
>> works already?
>
> Good point. I incorrectly assumed it would only be a minor
> contribution copyright-wise. > Mr Edelsohn gave me a template which I've
> now filled out and sent to assign@gnu.org. I'm assuming I just need to
> wait for them to send me the form. I'll update this thread when that's
> sorted. In the meantime I've hopefully fixed some of the issues.
Great.
>> Second, your patch was mangled by word wrap so that it can't be applied
>> without manual repair. If you can't prevent word wrap in your mail
>> client, please send it as an attachment rather than inline.
>
> Oh yes I see where it's gotten mangled now. I'm attaching it as a
> .patch file (I assume that's okay).
>
>> Also, there are a few whitespace issues in the patch; please run
>> contrib/check_GNU_style.sh on the patch before submitting.
>
> Should be all fixed now (there is one style issue left but it's a
> false positive). Visual Studio Code was lying to me about what the
> file looks like so if there are any more formatting issues please let
> me know.
>
>> If you use contrib/gcc-git-customization.sh and then git
>> gcc-commit-mklog you don't need to touch ChangeLog files at all, just
>> adjust the generated ChangeLog entries in the git commit message. I
>> personally tend to commit first with a placeholder message and then use
>> git gcc-commit-mklog --amend to generate the ChangeLog entries.
>
> Wouldn't that require read-write access? (Just from looking here
> https://gcc.gnu.org/gitwrite.html.)
You don't need write access to the main repository to use these commands
on your local copy. One nice thing about git compared to svn is that
you don't need to touch the server for anything but push and pull.
Incidentally, how are you producing your patch? Maybe try git
format-patch instead.
>> Probably. Can you use sort/uniq/diff on the .sum testsuite output to
>> determine which passes are missing in the patched sources?
>
> According to contrib/dg-cmp-results.sh ...
>
> I get a bunch of these weird NA->PASSes (and vice-versa), for example:
>
> PASS->NA: g++.dg/modules/alias-1_a.H module-cmi
> (gcm.cache/home/anthony/Desktop/GCC/builds_and_source/source_clean/gcc/testsuite/g++.dg/modules/alias-1_a.H.gcm)
> NA->PASS: g++.dg/modules/alias-1_a.H module-cmi
> (gcm.cache/home/anthony/Desktop/GCC/builds_and_source/source_pr17314/gcc/testsuite/g++.dg/modules/alias-1_a.H.gcm)
> PASS->NA: g++.dg/modules/alias-1_a.H module-cmi
> (gcm.cache/home/anthony/Desktop/GCC/builds_and_source/source_clean/gcc/testsuite/g++.dg/modules/alias-1_a.H.gcm)
> NA->PASS: g++.dg/modules/alias-1_a.H module-cmi
> (gcm.cache/home/anthony/Desktop/GCC/builds_and_source/source_pr17314/gcc/testsuite/g++.dg/modules/alias-1_a.H.gcm)
>
> They're weird because I haven't actually touched those files (so I'm
> assuming this is normal). There are about ~400 of those and they're
> all .gcm files. They seem to balance out.
The modules code and tests are very new and volatile, I wouldn't worry
about them.
> dr142.c reports:
>
> NA->PASS: g++.dg/tc1/dr142.C -std=c++14 (test for warnings, line 11)
> PASS->NA: g++.dg/tc1/dr142.C -std=c++14 (test for warnings, line 5)
> PASS->NA: g++.dg/tc1/dr142.C -std=c++14 (test for warnings, line 7)
> PASS->NA: g++.dg/tc1/dr142.C -std=c++14 (test for warnings, line 8)
> NA->PASS: g++.dg/tc1/dr142.C -std=c++17 (test for warnings, line 11)
> PASS->NA: g++.dg/tc1/dr142.C -std=c++17 (test for warnings, line 5)
> PASS->NA: g++.dg/tc1/dr142.C -std=c++17 (test for warnings, line 7)
> PASS->NA: g++.dg/tc1/dr142.C -std=c++17 (test for warnings, line 8)
> NA->PASS: g++.dg/tc1/dr142.C -std=c++2a (test for warnings, line 11)
> PASS->NA: g++.dg/tc1/dr142.C -std=c++2a (test for warnings, line 5)
> PASS->NA: g++.dg/tc1/dr142.C -std=c++2a (test for warnings, line 7)
> PASS->NA: g++.dg/tc1/dr142.C -std=c++2a (test for warnings, line 8)
> NA->PASS: g++.dg/tc1/dr142.C -std=c++98 (test for warnings, line 11)
> PASS->NA: g++.dg/tc1/dr142.C -std=c++98 (test for warnings, line 5)
> PASS->NA: g++.dg/tc1/dr142.C -std=c++98 (test for warnings, line 7)
> PASS->NA: g++.dg/tc1/dr142.C -std=c++98 (test for warnings, line 8)
These changes are because your patch changes that test to expect
warnings in different places.
> In other words, there are 12 PASS->NAs and 4 NA->PASSes in this file,
> meaning a net change of -8 (which explains why there are eight fewer).
> My other changes also report PASS->NAs and vice-versa, but for those
> the number of new NAs equals the number of new PASSes, so they don't
> cause a change in quantity.
>
> Thanks for being patient with me. I'll let you know when I've
> completed the forms.
>
> Also if I need to adjust the .patch to deal with the changelogs issue
> please let me know.
>
> Kind regards,
> Anthony
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] c++: private inheritance access diagnostics fix [PR17314]
2021-01-11 20:03 ` Jason Merrill
@ 2021-01-21 19:28 ` Anthony Sharp
2021-01-21 22:56 ` Jason Merrill
2021-01-22 3:44 ` Jason Merrill
0 siblings, 2 replies; 10+ messages in thread
From: Anthony Sharp @ 2021-01-21 19:28 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2164 bytes --]
Hi Jason,
I've finally completed my copyright assignment form. I've attached it
to this email for reference.
> You don't need write access to the main repository to use these commands
> on your local copy. One nice thing about git compared to svn is that
> you don't need to touch the server for anything but push and pull.
>
> Incidentally, how are you producing your patch? Maybe try git
> format-patch instead.
The method I am using at the moment is the one Ranjit Mathew talks
about here: http://rmathew.com/articles/gcj/crpatch.html. Actually,
having just re-read it, it says: 'NOTE: This is not the “proper” or
“official” way of creating and submitting patches - that process has
been explained in detail elsewhere. That process requires one to use
Subversion (SVN). The process described here is meant for “one-off
hackers” or people who cannot use SVN for some reason or the other.'
... oops!
It's my fault kind of - the official GCC webpage
(https://gcc.gnu.org/gitwrite.html) explaining how to do it is called
'Read-write Git access' so I assumed it was only relevant for people
who have access to the repo, but I see that is not the case.
I've tried the git way of doing it and I'm attaching a new patch file
that (hopefully) is better this time. Basically what I did was what
you suggested:
git pull
contrib/gcc-git-customization.sh
(make changes)
git add *
git gcc-commit-mklog
git gcc-commit-mklog --amend
git format-patch -1 master
I also re-built the source just to make sure I hadn't messed anything
up. I re-ran the C++ regression tests using make check-c and make
check-c++. Whilst I did not do a before/after comparison of the
results, I checked the FAILs in gcc.sum and g++.sum and they all
looked like they had nothing to do with my code. All the code is the
same as before, so I'm thinking it should be fine (I just wanted to be
safe). Also checked against check_GNU_style.sh.
Assuming that's all fine, as for the code itself, there might well be
some tweaks that could make it better, and so if that is the case then
please let me know.
Kind regards,
Anthony Sharp
[-- Attachment #2: Sharp.1672871.GCC.pdf --]
[-- Type: application/pdf, Size: 89622 bytes --]
[-- Attachment #3: pr17314_v2.patch --]
[-- Type: text/x-patch, Size: 16890 bytes --]
From 9f7fa0b4a892f717974d79a6a56a5f8a8a8d9943 Mon Sep 17 00:00:00 2001
From: Anthony Sharp <anthonysharp15@gmail.com>
Date: Thu, 21 Jan 2021 15:26:25 +0000
Subject: [PATCH] gcc/cp/ChangeLog:
2021-1-21 Anthony Sharp <anthonysharp15@gmail.com>
Fixes PR17314
* call.c (complain_about_access): Altered function.
* cp-tree.h (complain_about_access): Changed parameters of function.
* search.c (access_in_type): Made function non-static so it can be
used in semantics.c.
* semantics.c (access_in_type): Added as extern function.
(get_parent_with_private_access): Added function.
(enforce_access): Modified function.
* typeck.c (complain_about_unrecognized_member): Updated function
arguments in complain_about_access.
gcc/testsuite/ChangeLog:
2021-1-21 Anthony Sharp <anthonysharp15@gmail.com>
Changes required due to PR17314 fix
* g++.dg/lookup/scoped1.C: Modified testcase to run successfully with
changes.
* g++.dg/tc1/dr142.C: Same as above.
* g++.dg/tc1/dr52.C: Same as above.
* g++.old-deja/g++.brendan/visibility6.C: Same as above.
* g++.old-deja/g++.brendan/visibility8.C: Same as above.
* g++.old-deja/g++.jason/access8.C: Same as above.
* g++.old-deja/g++.law/access4.C: Same as above.
* g++.old-deja/g++.law/visibility12.C: Same as above.
* g++.old-deja/g++.law/visibility4.C: Same as above.
* g++.old-deja/g++.law/visibility8.C: Same as above.
* g++.old-deja/g++.other/access4.C: Same as above.
---
gcc/cp/call.c | 64 ++++++++++-------
gcc/cp/cp-tree.h | 3 +-
gcc/cp/search.c | 4 +-
gcc/cp/semantics.c | 68 ++++++++++++++++++-
gcc/cp/typeck.c | 3 +-
gcc/testsuite/g++.dg/lookup/scoped1.C | 4 +-
gcc/testsuite/g++.dg/tc1/dr142.C | 8 +--
gcc/testsuite/g++.dg/tc1/dr52.C | 6 +-
.../g++.old-deja/g++.brendan/visibility6.C | 4 +-
.../g++.old-deja/g++.brendan/visibility8.C | 4 +-
.../g++.old-deja/g++.jason/access8.C | 5 +-
gcc/testsuite/g++.old-deja/g++.law/access4.C | 5 +-
.../g++.old-deja/g++.law/visibility12.C | 7 +-
.../g++.old-deja/g++.law/visibility4.C | 5 +-
.../g++.old-deja/g++.law/visibility8.C | 5 +-
.../g++.old-deja/g++.other/access4.C | 4 +-
16 files changed, 145 insertions(+), 54 deletions(-)
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index b6e9f125aeb..fc2f1c6226c 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7142,33 +7142,51 @@ build_op_delete_call (enum tree_code code, tree addr, tree size,
/* Issue diagnostics about a disallowed access of DECL, using DIAG_DECL
in the diagnostics.
- If ISSUE_ERROR is true, then issue an error about the
- access, followed by a note showing the declaration.
- Otherwise, just show the note. */
+ If ISSUE_ERROR is true, then issue an error about the access, followed
+ by a note showing the declaration. Otherwise, just show the note.
+
+ DIAG_DECL and DIAG_LOCATION will almost always be the same.
+ DIAG_LOCATION is just another DECL. NO_ACCESS_REASON is an optional
+ parameter used to specify why DECL wasn't accessible (e.g. ak_private
+ would be because DECL was private). If not using NO_ACCESS_REASON,
+ then it must be ak_none, and the access failure reason will be
+ figured out by looking at the protection of DECL. */
void
-complain_about_access (tree decl, tree diag_decl, bool issue_error)
+complain_about_access (tree decl, tree diag_decl, tree diag_location,
+bool issue_error, access_kind no_access_reason)
{
- if (TREE_PRIVATE (decl))
- {
- if (issue_error)
- error ("%q#D is private within this context", diag_decl);
- inform (DECL_SOURCE_LOCATION (diag_decl),
- "declared private here");
- }
- else if (TREE_PROTECTED (decl))
- {
- if (issue_error)
- error ("%q#D is protected within this context", diag_decl);
- inform (DECL_SOURCE_LOCATION (diag_decl),
- "declared protected here");
- }
+ /* If we have not already figured out why DECL is innaccessible... */
+ if (no_access_reason == ak_none)
+ {
+ /* Examine the access of DECL to find out why. */
+ if (TREE_PRIVATE (decl))
+ no_access_reason = ak_private;
+ else if (TREE_PROTECTED (decl))
+ no_access_reason = ak_protected;
+ }
+
+ /* Now generate an error message depending on calculated access. */
+ if (no_access_reason == ak_private)
+ {
+ if (issue_error)
+ error ("%q#D is private within this context", diag_decl);
+ inform (DECL_SOURCE_LOCATION (diag_location), "declared private here");
+ }
+ else if (no_access_reason == ak_protected)
+ {
+ if (issue_error)
+ error ("%q#D is protected within this context", diag_decl);
+ inform (DECL_SOURCE_LOCATION (diag_location), "declared protected here");
+ }
+ /* Couldn't figure out why DECL is innaccesible, so just say it's
+ innaccessible. */
else
- {
- if (issue_error)
- error ("%q#D is inaccessible within this context", diag_decl);
- inform (DECL_SOURCE_LOCATION (diag_decl), "declared here");
- }
+ {
+ if (issue_error)
+ error ("%q#D is inaccessible within this context", diag_decl);
+ inform (DECL_SOURCE_LOCATION (diag_decl), "declared here");
+ }
}
/* Initialize a temporary of type TYPE with EXPR. The FLAGS are a
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 51139f4a4be..130f91b0ed2 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6434,7 +6434,8 @@ class access_failure_info
tree m_diag_decl;
};
-extern void complain_about_access (tree, tree, bool);
+extern void complain_about_access (tree, tree, tree, bool,
+ access_kind);
extern void push_defarg_context (tree);
extern void pop_defarg_context (void);
extern tree convert_default_arg (tree, tree, tree, int,
diff --git a/gcc/cp/search.c b/gcc/cp/search.c
index dd3773da4f7..6cb881a20c2 100644
--- a/gcc/cp/search.c
+++ b/gcc/cp/search.c
@@ -47,7 +47,7 @@ static tree dfs_walk_once_accessible (tree, bool,
tree (*post_fn) (tree, void *),
void *data);
static tree dfs_access_in_type (tree, void *);
-static access_kind access_in_type (tree, tree);
+access_kind access_in_type (tree, tree);
static tree dfs_get_pure_virtuals (tree, void *);
\f
@@ -583,7 +583,7 @@ dfs_access_in_type (tree binfo, void *data)
/* Return the access to DECL in TYPE. */
-static access_kind
+access_kind
access_in_type (tree type, tree decl)
{
tree binfo = TYPE_BINFO (type);
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 244fc70d02d..a576489e3b9 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -46,6 +46,8 @@ along with GCC; see the file COPYING3. If not see
#include "predict.h"
#include "memmodel.h"
+extern access_kind access_in_type (tree type, tree decl);
+
/* There routines provide a modular interface to perform many parsing
operations. They may therefore be used during actual parsing, or
during template instantiation, which may be regarded as a
@@ -256,6 +258,41 @@ pop_to_parent_deferring_access_checks (void)
}
}
+/* This deals with bug PR17314.
+
+ DECL is a declaration and BINFO represents a class that has attempted (but
+ failed) to access DECL.
+
+ Examine the parent binfos of BINFO and determine whether any of them had
+ private access to DECL. If they did, return the parent binfo. This helps
+ in figuring out the correct error message to show (if the parents had
+ access, it's their fault for not giving sufficient access to BINFO).
+
+ If no parents had access, return NULL_TREE. */
+
+static tree
+get_parent_with_private_access (tree decl, tree binfo)
+{
+ /* Only BINFOs should come through here. */
+ gcc_assert (TREE_CODE (binfo) == TREE_BINFO);
+
+ tree base_binfo = NULL_TREE;
+
+ /* Iterate through immediate parent classes. */
+ for (int i = 0; BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
+ {
+ /* This parent had private access. Therefore that's why BINFO can't
+ access DECL. */
+ if (access_in_type (BINFO_TYPE (base_binfo), decl) == ak_private)
+ return base_binfo;
+ }
+
+ /* None of the parents had access. Note: it's impossible for one of the
+ parents to have had public or protected access to DECL, since then
+ BINFO would have been able to access DECL too. */
+ return NULL_TREE;
+}
+
/* If the current scope isn't allowed to access DECL along
BASETYPE_PATH, give an error, or if we're parsing a function or class
template, defer the access check to be performed at instantiation time.
@@ -316,7 +353,36 @@ enforce_access (tree basetype_path, tree decl, tree diag_decl,
if (flag_new_inheriting_ctors)
diag_decl = strip_inheriting_ctors (diag_decl);
if (complain & tf_error)
- complain_about_access (decl, diag_decl, true);
+ {
+ /* We will usually want to point to the same place as
+ diag_decl but not always. */
+ tree diag_location = diag_decl;
+ access_kind parent_access = ak_none;
+
+ /* See if any of BASETYPE_PATH's parents had private access
+ to DECL. If they did, that will tell us why we don't. */
+ tree parent_binfo = get_parent_with_private_access (decl,
+ basetype_path);
+
+ /* If a parent had private access, then the diagnostic
+ location DECL should be that of the parent class, since it
+ failed to give suitable access by using a private
+ inheritance. But if DECL was actually defined in the parent,
+ it wasn't privately inherited, and so we don't need to do
+ this, and complain_about_access will figure out what to
+ do. */
+ if (parent_binfo != NULL_TREE
+ && context_for_name_lookup (decl)
+ != BINFO_TYPE (parent_binfo))
+ {
+ diag_location = TYPE_NAME (BINFO_TYPE (parent_binfo));
+ parent_access = ak_private;
+ }
+
+ /* Finally, generate an error message. */
+ complain_about_access (decl, diag_decl, diag_location, true,
+ parent_access);
+ }
if (afi)
afi->record_access_failure (basetype_path, decl, diag_decl);
return false;
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index cd592fd558e..0b3fd3785ae 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -2980,7 +2980,8 @@ complain_about_unrecognized_member (tree access_path, tree name,
TREE_CODE (access_path) == TREE_BINFO
? TREE_TYPE (access_path) : object_type,
name, afi.get_diag_decl ());
- complain_about_access (afi.get_decl (), afi.get_diag_decl (), false);
+ complain_about_access (afi.get_decl (), afi.get_diag_decl (),
+ afi.get_diag_decl (), false, ak_none);
}
}
else
diff --git a/gcc/testsuite/g++.dg/lookup/scoped1.C b/gcc/testsuite/g++.dg/lookup/scoped1.C
index 663f718b734..2f7f603b49f 100644
--- a/gcc/testsuite/g++.dg/lookup/scoped1.C
+++ b/gcc/testsuite/g++.dg/lookup/scoped1.C
@@ -4,12 +4,12 @@
struct A
{
static int i1;
- int i2; // { dg-message "declared" }
+ int i2;
static void f1 ();
void f2 ();
};
-struct B: private A { };
+struct B: private A { }; // { dg-message "declared" }
struct C: public B
{
void g ()
diff --git a/gcc/testsuite/g++.dg/tc1/dr142.C b/gcc/testsuite/g++.dg/tc1/dr142.C
index 2f0370233e6..6e216da89f0 100644
--- a/gcc/testsuite/g++.dg/tc1/dr142.C
+++ b/gcc/testsuite/g++.dg/tc1/dr142.C
@@ -2,13 +2,13 @@
// Origin: Giovanni Bajo <giovannibajo at gcc dot gnu dot org>
// DR142: Injection-related errors in access example
-class B { // { dg-message "declared" }
+class B {
public:
- int mi; // { dg-message "declared" }
- static int si; // { dg-message "declared" }
+ int mi;
+ static int si;
};
-class D: private B {
+class D: private B { // { dg-message "declared" }
};
class DD: public D {
diff --git a/gcc/testsuite/g++.dg/tc1/dr52.C b/gcc/testsuite/g++.dg/tc1/dr52.C
index 7cff847023f..17b6496916c 100644
--- a/gcc/testsuite/g++.dg/tc1/dr52.C
+++ b/gcc/testsuite/g++.dg/tc1/dr52.C
@@ -17,11 +17,11 @@ struct B1 : B {};
struct B2 : B {};
struct C
-{ // { dg-message "declared" }
- void foo(void);
+{
+ void foo(void);
};
-struct D : private C {};
+struct D : private C {}; // { dg-message "declared" }
struct X: A, B1, B2, D
{
diff --git a/gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C b/gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C
index 3dfaf7fd0ea..8d6c6f10806 100644
--- a/gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C
+++ b/gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C
@@ -3,9 +3,9 @@
class bottom
{
public:
- int b; // { dg-message "" } private
+ int b;
};
-class middle : private bottom
+class middle : private bottom // { dg-message "" } private
{
public:
void foo () { b; }
diff --git a/gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C b/gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C
index 3c443afe678..c165b0874a0 100644
--- a/gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C
+++ b/gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C
@@ -5,9 +5,9 @@
class foo
{
public:
- static int y; // { dg-message "" } private
+ static int y;
};
-class foo1 : private foo
+class foo1 : private foo // { dg-message "" } private
{ };
class foo2 : public foo1
{ public:
diff --git a/gcc/testsuite/g++.old-deja/g++.jason/access8.C b/gcc/testsuite/g++.old-deja/g++.jason/access8.C
index 4404d8ad95d..0aa85d0457c 100644
--- a/gcc/testsuite/g++.old-deja/g++.jason/access8.C
+++ b/gcc/testsuite/g++.old-deja/g++.jason/access8.C
@@ -3,13 +3,14 @@
// Date: 25 Jan 1994 23:41:33 -0500
// Bug: g++ forgets access decls after the definition.
-class inh { // { dg-message "" } inaccessible
+class inh {
int a;
protected:
void myf(int);
};
-class mel : private inh {
+class mel : private inh // { dg-message "" } inaccessible
+{
protected:
int t;
inh::myf; // { dg-warning "deprecated" }
diff --git a/gcc/testsuite/g++.old-deja/g++.law/access4.C b/gcc/testsuite/g++.old-deja/g++.law/access4.C
index 54072ce3b32..57fa24a0dde 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/access4.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/access4.C
@@ -6,12 +6,13 @@
// Subject: g++ 2.5.5 doesn't warn about inaccessible virtual base ctor
// Message-ID: <9403030024.AA04534@ses.com>
-class ForceLeafSterile { // { dg-message "" }
+class ForceLeafSterile {
friend class Sterile;
ForceLeafSterile() {} // { dg-message "" }
};
-class Sterile : private virtual ForceLeafSterile {
+class Sterile : private virtual ForceLeafSterile // { dg-message "" }
+{
public:
Sterile() {}
Sterile(const char* /*blah*/) {}
diff --git a/gcc/testsuite/g++.old-deja/g++.law/visibility12.C b/gcc/testsuite/g++.old-deja/g++.law/visibility12.C
index 59467ba7d80..6b7ff75d2ca 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/visibility12.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/visibility12.C
@@ -6,11 +6,12 @@
// Subject: member access rule bug
// Message-ID: <9306300528.AA17185@coda.mel.dit.CSIRO.AU>
struct a {
- int aa; // { dg-message "" } private
+ int aa;
};
-class b : private a {
- };
+class b : private a // { dg-message "" } private
+{
+};
class c : public b {
int xx(void) { return (aa); } // aa should be invisible// { dg-error "" } .*
diff --git a/gcc/testsuite/g++.old-deja/g++.law/visibility4.C b/gcc/testsuite/g++.old-deja/g++.law/visibility4.C
index 1cdec1c2b55..644154e66fb 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/visibility4.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/visibility4.C
@@ -8,10 +8,11 @@
class A {
public:
- int b; // { dg-message "" } private
+ int b;
};
-class C : private A { // NOTE WELL. private, not public
+class C : private A // { dg-message "" } private
+{
public:
int d;
};
diff --git a/gcc/testsuite/g++.old-deja/g++.law/visibility8.C b/gcc/testsuite/g++.old-deja/g++.law/visibility8.C
index 5242dfc804f..4457ddf46c7 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/visibility8.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/visibility8.C
@@ -7,11 +7,12 @@
// Message-ID: <m0nof3E-0021ifC@jts.com
class t1 {
protected:
- int a; // { dg-message "" } protected
+ int a;
};
-class t2 : private t1 {
+class t2 : private t1 // { dg-message "" } protected
+{
public:
int b;
};
diff --git a/gcc/testsuite/g++.old-deja/g++.other/access4.C b/gcc/testsuite/g++.old-deja/g++.other/access4.C
index d3c8d85c3f5..6c47700db19 100644
--- a/gcc/testsuite/g++.old-deja/g++.other/access4.C
+++ b/gcc/testsuite/g++.old-deja/g++.other/access4.C
@@ -1,10 +1,10 @@
// { dg-do assemble }
-struct A { // { dg-message "" } inaccessible
+struct A {
static int i;
};
-struct B : private A { };
+struct B : private A { }; // { dg-message "" } inaccessible
struct C : public B {
int f () { return A::i; } // { dg-error "" } context
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] c++: private inheritance access diagnostics fix [PR17314]
2021-01-21 19:28 ` Anthony Sharp
@ 2021-01-21 22:56 ` Jason Merrill
2021-01-22 3:44 ` Jason Merrill
1 sibling, 0 replies; 10+ messages in thread
From: Jason Merrill @ 2021-01-21 22:56 UTC (permalink / raw)
To: Anthony Sharp; +Cc: gcc-patches
On 1/21/21 2:28 PM, Anthony Sharp wrote:
> Hi Jason,
>
> I've finally completed my copyright assignment form. I've attached it
> to this email for reference.
>
>> You don't need write access to the main repository to use these commands
>> on your local copy. One nice thing about git compared to svn is that
>> you don't need to touch the server for anything but push and pull.
>>
>> Incidentally, how are you producing your patch? Maybe try git
>> format-patch instead.
>
> The method I am using at the moment is the one Ranjit Mathew talks
> about here: http://rmathew.com/articles/gcj/crpatch.html.
Interesting. Yeah, that page is obsolete since the move to git.
> It's my fault kind of - the official GCC webpage
> (https://gcc.gnu.org/gitwrite.html) explaining how to do it is called
> 'Read-write Git access' so I assumed it was only relevant for people
> who have access to the repo, but I see that is not the case.
Those pages could definitely be more clearly organized.
> I've tried the git way of doing it and I'm attaching a new patch file
> that (hopefully) is better this time. Basically what I did was what
> you suggested:
>
> git pull
> contrib/gcc-git-customization.sh
> (make changes)
> git add *
> git gcc-commit-mklog
> git gcc-commit-mklog --amend
Why two gcc-comit-mklog? That would generate the log entries twice.
You should also git gcc-verify at this point; for me, it complains about
some of your header lines in the log. Your author line needs to start
at the first column, and use "01" for January instead of just "1". The
other explanatory lines can be omitted, in favor of:
The commit message before the log entries should include your rationale
for the patch (e.g. the first two paragraphs of your initial email).
> git format-patch -1 master
>
> I also re-built the source just to make sure I hadn't messed anything
> up. I re-ran the C++ regression tests using make check-c and make
> check-c++. Whilst I did not do a before/after comparison of the
> results, I checked the FAILs in gcc.sum and g++.sum and they all
> looked like they had nothing to do with my code. All the code is the
> same as before, so I'm thinking it should be fine (I just wanted to be
> safe). Also checked against check_GNU_style.sh.
>
> Assuming that's all fine, as for the code itself, there might well be
> some tweaks that could make it better, and so if that is the case then
> please let me know.
I'll look at the code soon.
Thanks,
Jason
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] c++: private inheritance access diagnostics fix [PR17314]
2021-01-21 19:28 ` Anthony Sharp
2021-01-21 22:56 ` Jason Merrill
@ 2021-01-22 3:44 ` Jason Merrill
2021-01-22 20:07 ` Anthony Sharp
1 sibling, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2021-01-22 3:44 UTC (permalink / raw)
To: Anthony Sharp; +Cc: gcc-patches
On 1/21/21 2:28 PM, Anthony Sharp wrote:
> Hi Jason,
>
> I've finally completed my copyright assignment form. I've attached it
> to this email for reference.
>
>> You don't need write access to the main repository to use these commands
>> on your local copy. One nice thing about git compared to svn is that
>> you don't need to touch the server for anything but push and pull.
>>
>> Incidentally, how are you producing your patch? Maybe try git
>> format-patch instead.
>
> The method I am using at the moment is the one Ranjit Mathew talks
> about here: http://rmathew.com/articles/gcj/crpatch.html. Actually,
> having just re-read it, it says: 'NOTE: This is not the “proper” or
> “official” way of creating and submitting patches - that process has
> been explained in detail elsewhere. That process requires one to use
> Subversion (SVN). The process described here is meant for “one-off
> hackers” or people who cannot use SVN for some reason or the other.'
> ... oops!
>
> It's my fault kind of - the official GCC webpage
> (https://gcc.gnu.org/gitwrite.html) explaining how to do it is called
> 'Read-write Git access' so I assumed it was only relevant for people
> who have access to the repo, but I see that is not the case.
>
> I've tried the git way of doing it and I'm attaching a new patch file
> that (hopefully) is better this time. Basically what I did was what
> you suggested:
>
> git pull
> contrib/gcc-git-customization.sh
> (make changes)
> git add *
> git gcc-commit-mklog
> git gcc-commit-mklog --amend
> git format-patch -1 master
>
> I also re-built the source just to make sure I hadn't messed anything
> up. I re-ran the C++ regression tests using make check-c and make
> check-c++. Whilst I did not do a before/after comparison of the
> results, I checked the FAILs in gcc.sum and g++.sum and they all
> looked like they had nothing to do with my code. All the code is the
> same as before, so I'm thinking it should be fine (I just wanted to be
> safe). Also checked against check_GNU_style.sh.
>
> Assuming that's all fine, as for the code itself, there might well be
> some tweaks that could make it better, and so if that is the case then
> please let me know.
The code looks good, I just have some minor tweaks. Thanks!
> +++ b/gcc/cp/semantics.c
...
> +extern access_kind access_in_type (tree type, tree decl);
...
> +static tree
> +get_parent_with_private_access (tree decl, tree binfo)
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).
> + /* If we have not already figured out why DECL is innaccessible... */
...
> + /* Couldn't figure out why DECL is innaccesible, so just say it's
> + innaccessible. */
Only one 'n' in inaccessible.
There are various minor formatting issues:
(https://www.gnu.org/prep/standards/standards.html#Formatting)
> + /* Couldn't figure out why DECL is innaccesible, so just say it's
> + innaccessible. */
Subsequent lines of a comment should be indented to line up with the
first line. This applies to all your multi-line comments.
> - {
> - if (issue_error)
> - error ("%q#D is private within this context", diag_decl);
> - inform (DECL_SOURCE_LOCATION (diag_decl),
> - "declared private here");
> - }
> + {
> + if (issue_error)
> + error ("%q#D is private within this context", diag_decl);
> + inform (DECL_SOURCE_LOCATION (diag_location), "declared private here");
> + }
Don't change the indentation of these blocks; in the GNU coding style
the { } are indented two spaces from the if.
> + tree parent_binfo = get_parent_with_private_access (decl,
> + basetype_path);
...
> + complain_about_access (decl, diag_decl, diag_location, true,
> + parent_access);
The new line of arguments should be indented to line up with the first one.
Jason
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] c++: private inheritance access diagnostics fix [PR17314]
2021-01-22 3:44 ` Jason Merrill
@ 2021-01-22 20:07 ` Anthony Sharp
2021-01-22 21:18 ` Jason Merrill
0 siblings, 1 reply; 10+ messages in thread
From: Anthony Sharp @ 2021-01-22 20:07 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1407 bytes --]
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.
Kind regards,
Anthony
[-- Attachment #2: pr17314_v3.patch --]
[-- Type: text/x-patch, Size: 16457 bytes --]
From 7984020f16e715017e62b8637d2e69c1aec3478a Mon Sep 17 00:00:00 2001
From: Anthony Sharp <anthonysharp15@gmail.com>
Date: Thu, 21 Jan 2021 15:26:25 +0000
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 reason this bug happened was because GCC was examining the
declared access of decl, instead of looking at it in the
context of class inheritance.
gcc/cp/ChangeLog:
2021-01-21 Anthony Sharp <anthonysharp15@gmail.com>
* call.c (complain_about_access): Altered function.
* cp-tree.h (complain_about_access): Changed parameters of function.
(get_parent_with_private_access): Declared new function.
* search.c (get_parent_with_private_access): Defined new function.
* semantics.c (enforce_access): Modified function.
* typeck.c (complain_about_unrecognized_member): Updated function
arguments in complain_about_access.
gcc/testsuite/ChangeLog:
2021-01-21 Anthony Sharp <anthonysharp15@gmail.com>
* g++.dg/lookup/scoped1.C: Modified testcase to run successfully with
changes.
* g++.dg/tc1/dr142.C: Same as above.
* g++.dg/tc1/dr52.C: Same as above.
* g++.old-deja/g++.brendan/visibility6.C: Same as above.
* g++.old-deja/g++.brendan/visibility8.C: Same as above.
* g++.old-deja/g++.jason/access8.C: Same as above.
* g++.old-deja/g++.law/access4.C: Same as above.
* g++.old-deja/g++.law/visibility12.C: Same as above.
* g++.old-deja/g++.law/visibility4.C: Same as above.
* g++.old-deja/g++.law/visibility8.C: Same as above.
* g++.old-deja/g++.other/access4.C: Same as above.
---
gcc/cp/call.c | 38 ++++++++++++++-----
gcc/cp/cp-tree.h | 4 +-
gcc/cp/search.c | 35 +++++++++++++++++
gcc/cp/semantics.c | 31 ++++++++++++++-
gcc/cp/typeck.c | 3 +-
gcc/testsuite/g++.dg/lookup/scoped1.C | 4 +-
gcc/testsuite/g++.dg/tc1/dr142.C | 8 ++--
gcc/testsuite/g++.dg/tc1/dr52.C | 6 +--
.../g++.old-deja/g++.brendan/visibility6.C | 4 +-
.../g++.old-deja/g++.brendan/visibility8.C | 4 +-
.../g++.old-deja/g++.jason/access8.C | 5 ++-
gcc/testsuite/g++.old-deja/g++.law/access4.C | 5 ++-
.../g++.old-deja/g++.law/visibility12.C | 7 ++--
.../g++.old-deja/g++.law/visibility4.C | 5 ++-
.../g++.old-deja/g++.law/visibility8.C | 5 ++-
.../g++.old-deja/g++.other/access4.C | 4 +-
16 files changed, 129 insertions(+), 39 deletions(-)
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index b6e9f125aeb..175a3d9b421 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7142,27 +7142,45 @@ build_op_delete_call (enum tree_code code, tree addr, tree size,
/* Issue diagnostics about a disallowed access of DECL, using DIAG_DECL
in the diagnostics.
- If ISSUE_ERROR is true, then issue an error about the
- access, followed by a note showing the declaration.
- Otherwise, just show the note. */
+ If ISSUE_ERROR is true, then issue an error about the access, followed
+ by a note showing the declaration. Otherwise, just show the note.
+
+ DIAG_DECL and DIAG_LOCATION will almost always be the same.
+ DIAG_LOCATION is just another DECL. NO_ACCESS_REASON is an optional
+ parameter used to specify why DECL wasn't accessible (e.g. ak_private
+ would be because DECL was private). If not using NO_ACCESS_REASON,
+ then it must be ak_none, and the access failure reason will be
+ figured out by looking at the protection of DECL. */
void
-complain_about_access (tree decl, tree diag_decl, bool issue_error)
+complain_about_access (tree decl, tree diag_decl, tree diag_location,
+bool issue_error, access_kind no_access_reason)
{
- if (TREE_PRIVATE (decl))
+ /* If we have not already figured out why DECL is inaccessible... */
+ if (no_access_reason == ak_none)
+ {
+ /* Examine the access of DECL to find out why. */
+ if (TREE_PRIVATE (decl))
+ no_access_reason = ak_private;
+ else if (TREE_PROTECTED (decl))
+ no_access_reason = ak_protected;
+ }
+
+ /* Now generate an error message depending on calculated access. */
+ if (no_access_reason == ak_private)
{
if (issue_error)
error ("%q#D is private within this context", diag_decl);
- inform (DECL_SOURCE_LOCATION (diag_decl),
- "declared private here");
+ inform (DECL_SOURCE_LOCATION (diag_location), "declared private here");
}
- else if (TREE_PROTECTED (decl))
+ else if (no_access_reason == ak_protected)
{
if (issue_error)
error ("%q#D is protected within this context", diag_decl);
- inform (DECL_SOURCE_LOCATION (diag_decl),
- "declared protected here");
+ inform (DECL_SOURCE_LOCATION (diag_location), "declared protected here");
}
+ /* Couldn't figure out why DECL is inaccesible, so just say it's
+ inaccessible. */
else
{
if (issue_error)
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 51139f4a4be..d53d98d2284 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6434,7 +6434,8 @@ class access_failure_info
tree m_diag_decl;
};
-extern void complain_about_access (tree, tree, bool);
+extern void complain_about_access (tree, tree, tree, bool,
+ access_kind);
extern void push_defarg_context (tree);
extern void pop_defarg_context (void);
extern tree convert_default_arg (tree, tree, tree, int,
@@ -7274,6 +7275,7 @@ extern unsigned get_pseudo_tinfo_index (tree);
extern tree get_pseudo_tinfo_type (unsigned);
/* in search.c */
+extern tree get_parent_with_private_access (tree decl, tree binfo);
extern bool accessible_base_p (tree, tree, bool);
extern tree lookup_base (tree, tree, base_access,
base_kind *, tsubst_flags_t);
diff --git a/gcc/cp/search.c b/gcc/cp/search.c
index dd3773da4f7..46a7ecb144d 100644
--- a/gcc/cp/search.c
+++ b/gcc/cp/search.c
@@ -122,6 +122,41 @@ dfs_lookup_base (tree binfo, void *data_)
return NULL_TREE;
}
+/* This deals with bug PR17314.
+
+ DECL is a declaration and BINFO represents a class that has attempted (but
+ failed) to access DECL.
+
+ Examine the parent binfos of BINFO and determine whether any of them had
+ private access to DECL. If they did, return the parent binfo. This helps
+ in figuring out the correct error message to show (if the parents had
+ access, it's their fault for not giving sufficient access to BINFO).
+
+ If no parents had access, return NULL_TREE. */
+
+tree
+get_parent_with_private_access (tree decl, tree binfo)
+{
+ /* Only BINFOs should come through here. */
+ gcc_assert (TREE_CODE (binfo) == TREE_BINFO);
+
+ tree base_binfo = NULL_TREE;
+
+ /* Iterate through immediate parent classes. */
+ for (int i = 0; BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
+ {
+ /* This parent had private access. Therefore that's why BINFO can't
+ access DECL. */
+ if (access_in_type (BINFO_TYPE (base_binfo), decl) == ak_private)
+ return base_binfo;
+ }
+
+ /* None of the parents had access. Note: it's impossible for one of the
+ parents to have had public or protected access to DECL, since then
+ BINFO would have been able to access DECL too. */
+ return NULL_TREE;
+}
+
/* Returns true if type BASE is accessible in T. (BASE is known to be
a (possibly non-proper) base class of T.) If CONSIDER_LOCAL_P is
true, consider any special access of the current scope, or access
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 244fc70d02d..13d14334fdd 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -316,7 +316,36 @@ enforce_access (tree basetype_path, tree decl, tree diag_decl,
if (flag_new_inheriting_ctors)
diag_decl = strip_inheriting_ctors (diag_decl);
if (complain & tf_error)
- complain_about_access (decl, diag_decl, true);
+ {
+ /* We will usually want to point to the same place as
+ diag_decl but not always. */
+ tree diag_location = diag_decl;
+ access_kind parent_access = ak_none;
+
+ /* See if any of BASETYPE_PATH's parents had private access
+ to DECL. If they did, that will tell us why we don't. */
+ tree parent_binfo = get_parent_with_private_access (decl,
+ basetype_path);
+
+ /* If a parent had private access, then the diagnostic
+ location DECL should be that of the parent class, since it
+ failed to give suitable access by using a private
+ inheritance. But if DECL was actually defined in the parent,
+ it wasn't privately inherited, and so we don't need to do
+ this, and complain_about_access will figure out what to
+ do. */
+ if (parent_binfo != NULL_TREE
+ && context_for_name_lookup (decl)
+ != BINFO_TYPE (parent_binfo))
+ {
+ diag_location = TYPE_NAME (BINFO_TYPE (parent_binfo));
+ parent_access = ak_private;
+ }
+
+ /* Finally, generate an error message. */
+ complain_about_access (decl, diag_decl, diag_location, true,
+ parent_access);
+ }
if (afi)
afi->record_access_failure (basetype_path, decl, diag_decl);
return false;
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index cd592fd558e..38a26227569 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -2980,7 +2980,8 @@ complain_about_unrecognized_member (tree access_path, tree name,
TREE_CODE (access_path) == TREE_BINFO
? TREE_TYPE (access_path) : object_type,
name, afi.get_diag_decl ());
- complain_about_access (afi.get_decl (), afi.get_diag_decl (), false);
+ complain_about_access (afi.get_decl (), afi.get_diag_decl (),
+ afi.get_diag_decl (), false, ak_none);
}
}
else
diff --git a/gcc/testsuite/g++.dg/lookup/scoped1.C b/gcc/testsuite/g++.dg/lookup/scoped1.C
index 663f718b734..2f7f603b49f 100644
--- a/gcc/testsuite/g++.dg/lookup/scoped1.C
+++ b/gcc/testsuite/g++.dg/lookup/scoped1.C
@@ -4,12 +4,12 @@
struct A
{
static int i1;
- int i2; // { dg-message "declared" }
+ int i2;
static void f1 ();
void f2 ();
};
-struct B: private A { };
+struct B: private A { }; // { dg-message "declared" }
struct C: public B
{
void g ()
diff --git a/gcc/testsuite/g++.dg/tc1/dr142.C b/gcc/testsuite/g++.dg/tc1/dr142.C
index 2f0370233e6..6e216da89f0 100644
--- a/gcc/testsuite/g++.dg/tc1/dr142.C
+++ b/gcc/testsuite/g++.dg/tc1/dr142.C
@@ -2,13 +2,13 @@
// Origin: Giovanni Bajo <giovannibajo at gcc dot gnu dot org>
// DR142: Injection-related errors in access example
-class B { // { dg-message "declared" }
+class B {
public:
- int mi; // { dg-message "declared" }
- static int si; // { dg-message "declared" }
+ int mi;
+ static int si;
};
-class D: private B {
+class D: private B { // { dg-message "declared" }
};
class DD: public D {
diff --git a/gcc/testsuite/g++.dg/tc1/dr52.C b/gcc/testsuite/g++.dg/tc1/dr52.C
index 7cff847023f..17b6496916c 100644
--- a/gcc/testsuite/g++.dg/tc1/dr52.C
+++ b/gcc/testsuite/g++.dg/tc1/dr52.C
@@ -17,11 +17,11 @@ struct B1 : B {};
struct B2 : B {};
struct C
-{ // { dg-message "declared" }
- void foo(void);
+{
+ void foo(void);
};
-struct D : private C {};
+struct D : private C {}; // { dg-message "declared" }
struct X: A, B1, B2, D
{
diff --git a/gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C b/gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C
index 3dfaf7fd0ea..8d6c6f10806 100644
--- a/gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C
+++ b/gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C
@@ -3,9 +3,9 @@
class bottom
{
public:
- int b; // { dg-message "" } private
+ int b;
};
-class middle : private bottom
+class middle : private bottom // { dg-message "" } private
{
public:
void foo () { b; }
diff --git a/gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C b/gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C
index 3c443afe678..c165b0874a0 100644
--- a/gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C
+++ b/gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C
@@ -5,9 +5,9 @@
class foo
{
public:
- static int y; // { dg-message "" } private
+ static int y;
};
-class foo1 : private foo
+class foo1 : private foo // { dg-message "" } private
{ };
class foo2 : public foo1
{ public:
diff --git a/gcc/testsuite/g++.old-deja/g++.jason/access8.C b/gcc/testsuite/g++.old-deja/g++.jason/access8.C
index 4404d8ad95d..0aa85d0457c 100644
--- a/gcc/testsuite/g++.old-deja/g++.jason/access8.C
+++ b/gcc/testsuite/g++.old-deja/g++.jason/access8.C
@@ -3,13 +3,14 @@
// Date: 25 Jan 1994 23:41:33 -0500
// Bug: g++ forgets access decls after the definition.
-class inh { // { dg-message "" } inaccessible
+class inh {
int a;
protected:
void myf(int);
};
-class mel : private inh {
+class mel : private inh // { dg-message "" } inaccessible
+{
protected:
int t;
inh::myf; // { dg-warning "deprecated" }
diff --git a/gcc/testsuite/g++.old-deja/g++.law/access4.C b/gcc/testsuite/g++.old-deja/g++.law/access4.C
index 54072ce3b32..57fa24a0dde 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/access4.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/access4.C
@@ -6,12 +6,13 @@
// Subject: g++ 2.5.5 doesn't warn about inaccessible virtual base ctor
// Message-ID: <9403030024.AA04534@ses.com>
-class ForceLeafSterile { // { dg-message "" }
+class ForceLeafSterile {
friend class Sterile;
ForceLeafSterile() {} // { dg-message "" }
};
-class Sterile : private virtual ForceLeafSterile {
+class Sterile : private virtual ForceLeafSterile // { dg-message "" }
+{
public:
Sterile() {}
Sterile(const char* /*blah*/) {}
diff --git a/gcc/testsuite/g++.old-deja/g++.law/visibility12.C b/gcc/testsuite/g++.old-deja/g++.law/visibility12.C
index 59467ba7d80..6b7ff75d2ca 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/visibility12.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/visibility12.C
@@ -6,11 +6,12 @@
// Subject: member access rule bug
// Message-ID: <9306300528.AA17185@coda.mel.dit.CSIRO.AU>
struct a {
- int aa; // { dg-message "" } private
+ int aa;
};
-class b : private a {
- };
+class b : private a // { dg-message "" } private
+{
+};
class c : public b {
int xx(void) { return (aa); } // aa should be invisible// { dg-error "" } .*
diff --git a/gcc/testsuite/g++.old-deja/g++.law/visibility4.C b/gcc/testsuite/g++.old-deja/g++.law/visibility4.C
index 1cdec1c2b55..644154e66fb 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/visibility4.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/visibility4.C
@@ -8,10 +8,11 @@
class A {
public:
- int b; // { dg-message "" } private
+ int b;
};
-class C : private A { // NOTE WELL. private, not public
+class C : private A // { dg-message "" } private
+{
public:
int d;
};
diff --git a/gcc/testsuite/g++.old-deja/g++.law/visibility8.C b/gcc/testsuite/g++.old-deja/g++.law/visibility8.C
index 5242dfc804f..4457ddf46c7 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/visibility8.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/visibility8.C
@@ -7,11 +7,12 @@
// Message-ID: <m0nof3E-0021ifC@jts.com
class t1 {
protected:
- int a; // { dg-message "" } protected
+ int a;
};
-class t2 : private t1 {
+class t2 : private t1 // { dg-message "" } protected
+{
public:
int b;
};
diff --git a/gcc/testsuite/g++.old-deja/g++.other/access4.C b/gcc/testsuite/g++.old-deja/g++.other/access4.C
index d3c8d85c3f5..6c47700db19 100644
--- a/gcc/testsuite/g++.old-deja/g++.other/access4.C
+++ b/gcc/testsuite/g++.old-deja/g++.other/access4.C
@@ -1,10 +1,10 @@
// { dg-do assemble }
-struct A { // { dg-message "" } inaccessible
+struct A {
static int i;
};
-struct B : private A { };
+struct B : private A { }; // { dg-message "" } inaccessible
struct C : public B {
int f () { return A::i; } // { dg-error "" } context
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] c++: private inheritance access diagnostics fix [PR17314]
2021-01-22 20:07 ` Anthony Sharp
@ 2021-01-22 21:18 ` Jason Merrill
2021-01-22 22:36 ` Anthony Sharp
0 siblings, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2021-01-22 21:18 UTC (permalink / raw)
To: Anthony Sharp; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] c++: private inheritance access diagnostics fix [PR17314]
2021-01-22 21:18 ` Jason Merrill
@ 2021-01-22 22:36 ` Anthony Sharp
0 siblings, 0 replies; 10+ messages in thread
From: Anthony Sharp @ 2021-01-22 22:36 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 91 bytes --]
Hi Jason,
Attached changes. I just edited the patch file directly.
Kind regards,
Anthony
[-- Attachment #2: pr17314_v4.patch --]
[-- Type: text/x-patch, Size: 16506 bytes --]
From 7984020f16e715017e62b8637d2e69c1aec3478a Mon Sep 17 00:00:00 2001
From: Anthony Sharp <anthonysharp15@gmail.com>
Date: Thu, 21 Jan 2021 15:26:25 +0000
Subject: [PATCH] c++: Private inheritance access diagnostics fix [PR17314]
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 reason this bug happened was because GCC was examining the
declared access of decl, instead of looking at it in the
context of class inheritance.
gcc/cp/ChangeLog:
2021-01-21 Anthony Sharp <anthonysharp15@gmail.com>
* call.c (complain_about_access): Altered function.
* cp-tree.h (complain_about_access): Changed parameters of function.
(get_parent_with_private_access): Declared new function.
* search.c (get_parent_with_private_access): Defined new function.
* semantics.c (enforce_access): Modified function.
* typeck.c (complain_about_unrecognized_member): Updated function
arguments in complain_about_access.
gcc/testsuite/ChangeLog:
2021-01-21 Anthony Sharp <anthonysharp15@gmail.com>
* g++.dg/lookup/scoped1.C: Modified testcase to run successfully with
changes.
* g++.dg/tc1/dr142.C: Same as above.
* g++.dg/tc1/dr52.C: Same as above.
* g++.old-deja/g++.brendan/visibility6.C: Same as above.
* g++.old-deja/g++.brendan/visibility8.C: Same as above.
* g++.old-deja/g++.jason/access8.C: Same as above.
* g++.old-deja/g++.law/access4.C: Same as above.
* g++.old-deja/g++.law/visibility12.C: Same as above.
* g++.old-deja/g++.law/visibility4.C: Same as above.
* g++.old-deja/g++.law/visibility8.C: Same as above.
* g++.old-deja/g++.other/access4.C: Same as above.
---
gcc/cp/call.c | 38 ++++++++++++++-----
gcc/cp/cp-tree.h | 4 +-
gcc/cp/search.c | 35 +++++++++++++++++
gcc/cp/semantics.c | 31 ++++++++++++++-
gcc/cp/typeck.c | 3 +-
gcc/testsuite/g++.dg/lookup/scoped1.C | 4 +-
gcc/testsuite/g++.dg/tc1/dr142.C | 8 ++--
gcc/testsuite/g++.dg/tc1/dr52.C | 6 +--
.../g++.old-deja/g++.brendan/visibility6.C | 4 +-
.../g++.old-deja/g++.brendan/visibility8.C | 4 +-
.../g++.old-deja/g++.jason/access8.C | 5 ++-
gcc/testsuite/g++.old-deja/g++.law/access4.C | 5 ++-
.../g++.old-deja/g++.law/visibility12.C | 7 ++--
.../g++.old-deja/g++.law/visibility4.C | 5 ++-
.../g++.old-deja/g++.law/visibility8.C | 5 ++-
.../g++.old-deja/g++.other/access4.C | 4 +-
16 files changed, 129 insertions(+), 39 deletions(-)
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index b6e9f125aeb..175a3d9b421 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7142,27 +7142,45 @@ build_op_delete_call (enum tree_code code, tree addr, tree size,
/* Issue diagnostics about a disallowed access of DECL, using DIAG_DECL
in the diagnostics.
- If ISSUE_ERROR is true, then issue an error about the
- access, followed by a note showing the declaration.
- Otherwise, just show the note. */
+ If ISSUE_ERROR is true, then issue an error about the access, followed
+ by a note showing the declaration. Otherwise, just show the note.
+
+ DIAG_DECL and DIAG_LOCATION will almost always be the same.
+ DIAG_LOCATION is just another DECL. NO_ACCESS_REASON is an optional
+ parameter used to specify why DECL wasn't accessible (e.g. ak_private
+ would be because DECL was private). If not using NO_ACCESS_REASON,
+ then it must be ak_none, and the access failure reason will be
+ figured out by looking at the protection of DECL. */
void
-complain_about_access (tree decl, tree diag_decl, bool issue_error)
+complain_about_access (tree decl, tree diag_decl, tree diag_location,
+bool issue_error, access_kind no_access_reason)
{
- if (TREE_PRIVATE (decl))
+ /* If we have not already figured out why DECL is inaccessible... */
+ if (no_access_reason == ak_none)
+ {
+ /* Examine the access of DECL to find out why. */
+ if (TREE_PRIVATE (decl))
+ no_access_reason = ak_private;
+ else if (TREE_PROTECTED (decl))
+ no_access_reason = ak_protected;
+ }
+
+ /* Now generate an error message depending on calculated access. */
+ if (no_access_reason == ak_private)
{
if (issue_error)
error ("%q#D is private within this context", diag_decl);
- inform (DECL_SOURCE_LOCATION (diag_decl),
- "declared private here");
+ inform (DECL_SOURCE_LOCATION (diag_location), "declared private here");
}
- else if (TREE_PROTECTED (decl))
+ else if (no_access_reason == ak_protected)
{
if (issue_error)
error ("%q#D is protected within this context", diag_decl);
- inform (DECL_SOURCE_LOCATION (diag_decl),
- "declared protected here");
+ inform (DECL_SOURCE_LOCATION (diag_location), "declared protected here");
}
+ /* Couldn't figure out why DECL is inaccesible, so just say it's
+ inaccessible. */
else
{
if (issue_error)
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 51139f4a4be..d53d98d2284 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6434,7 +6434,8 @@ class access_failure_info
tree m_diag_decl;
};
-extern void complain_about_access (tree, tree, bool);
+extern void complain_about_access (tree, tree, tree, bool,
+ access_kind);
extern void push_defarg_context (tree);
extern void pop_defarg_context (void);
extern tree convert_default_arg (tree, tree, tree, int,
@@ -7274,6 +7275,7 @@ extern unsigned get_pseudo_tinfo_index (tree);
extern tree get_pseudo_tinfo_type (unsigned);
/* in search.c */
+extern tree get_parent_with_private_access (tree decl, tree binfo);
extern bool accessible_base_p (tree, tree, bool);
extern tree lookup_base (tree, tree, base_access,
base_kind *, tsubst_flags_t);
diff --git a/gcc/cp/search.c b/gcc/cp/search.c
index dd3773da4f7..46a7ecb144d 100644
--- a/gcc/cp/search.c
+++ b/gcc/cp/search.c
@@ -122,6 +122,41 @@ dfs_lookup_base (tree binfo, void *data_)
return NULL_TREE;
}
+/* This deals with bug PR17314.
+
+ DECL is a declaration and BINFO represents a class that has attempted (but
+ failed) to access DECL.
+
+ Examine the parent binfos of BINFO and determine whether any of them had
+ private access to DECL. If they did, return the parent binfo. This helps
+ in figuring out the correct error message to show (if the parents had
+ access, it's their fault for not giving sufficient access to BINFO).
+
+ If no parents had access, return NULL_TREE. */
+
+tree
+get_parent_with_private_access (tree decl, tree binfo)
+{
+ /* Only BINFOs should come through here. */
+ gcc_assert (TREE_CODE (binfo) == TREE_BINFO);
+
+ tree base_binfo = NULL_TREE;
+
+ /* Iterate through immediate parent classes. */
+ for (int i = 0; BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
+ {
+ /* This parent had private access. Therefore that's why BINFO can't
+ access DECL. */
+ if (access_in_type (BINFO_TYPE (base_binfo), decl) == ak_private)
+ return base_binfo;
+ }
+
+ /* None of the parents had access. Note: it's impossible for one of the
+ parents to have had public or protected access to DECL, since then
+ BINFO would have been able to access DECL too. */
+ return NULL_TREE;
+}
+
/* Returns true if type BASE is accessible in T. (BASE is known to be
a (possibly non-proper) base class of T.) If CONSIDER_LOCAL_P is
true, consider any special access of the current scope, or access
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 244fc70d02d..13d14334fdd 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -316,7 +316,36 @@ enforce_access (tree basetype_path, tree decl, tree diag_decl,
if (flag_new_inheriting_ctors)
diag_decl = strip_inheriting_ctors (diag_decl);
if (complain & tf_error)
- complain_about_access (decl, diag_decl, true);
+ {
+ /* We will usually want to point to the same place as
+ diag_decl but not always. */
+ tree diag_location = diag_decl;
+ access_kind parent_access = ak_none;
+
+ /* See if any of BASETYPE_PATH's parents had private access
+ to DECL. If they did, that will tell us why we don't. */
+ tree parent_binfo = get_parent_with_private_access (decl,
+ basetype_path);
+
+ /* If a parent had private access, then the diagnostic
+ location DECL should be that of the parent class, since it
+ failed to give suitable access by using a private
+ inheritance. But if DECL was actually defined in the parent,
+ it wasn't privately inherited, and so we don't need to do
+ this, and complain_about_access will figure out what to
+ do. */
+ if (parent_binfo != NULL_TREE
+ && (context_for_name_lookup (decl)
+ != BINFO_TYPE (parent_binfo)))
+ {
+ diag_location = TYPE_NAME (BINFO_TYPE (parent_binfo));
+ parent_access = ak_private;
+ }
+
+ /* Finally, generate an error message. */
+ complain_about_access (decl, diag_decl, diag_location, true,
+ parent_access);
+ }
if (afi)
afi->record_access_failure (basetype_path, decl, diag_decl);
return false;
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index cd592fd558e..38a26227569 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -2980,7 +2980,8 @@ complain_about_unrecognized_member (tree access_path, tree name,
TREE_CODE (access_path) == TREE_BINFO
? TREE_TYPE (access_path) : object_type,
name, afi.get_diag_decl ());
- complain_about_access (afi.get_decl (), afi.get_diag_decl (), false);
+ complain_about_access (afi.get_decl (), afi.get_diag_decl (),
+ afi.get_diag_decl (), false, ak_none);
}
}
else
diff --git a/gcc/testsuite/g++.dg/lookup/scoped1.C b/gcc/testsuite/g++.dg/lookup/scoped1.C
index 663f718b734..2f7f603b49f 100644
--- a/gcc/testsuite/g++.dg/lookup/scoped1.C
+++ b/gcc/testsuite/g++.dg/lookup/scoped1.C
@@ -4,12 +4,12 @@
struct A
{
static int i1;
- int i2; // { dg-message "declared" }
+ int i2;
static void f1 ();
void f2 ();
};
-struct B: private A { };
+struct B: private A { }; // { dg-message "declared" }
struct C: public B
{
void g ()
diff --git a/gcc/testsuite/g++.dg/tc1/dr142.C b/gcc/testsuite/g++.dg/tc1/dr142.C
index 2f0370233e6..6e216da89f0 100644
--- a/gcc/testsuite/g++.dg/tc1/dr142.C
+++ b/gcc/testsuite/g++.dg/tc1/dr142.C
@@ -2,13 +2,13 @@
// Origin: Giovanni Bajo <giovannibajo at gcc dot gnu dot org>
// DR142: Injection-related errors in access example
-class B { // { dg-message "declared" }
+class B {
public:
- int mi; // { dg-message "declared" }
- static int si; // { dg-message "declared" }
+ int mi;
+ static int si;
};
-class D: private B {
+class D: private B { // { dg-message "declared" }
};
class DD: public D {
diff --git a/gcc/testsuite/g++.dg/tc1/dr52.C b/gcc/testsuite/g++.dg/tc1/dr52.C
index 7cff847023f..17b6496916c 100644
--- a/gcc/testsuite/g++.dg/tc1/dr52.C
+++ b/gcc/testsuite/g++.dg/tc1/dr52.C
@@ -17,11 +17,11 @@ struct B1 : B {};
struct B2 : B {};
struct C
-{ // { dg-message "declared" }
- void foo(void);
+{
+ void foo(void);
};
-struct D : private C {};
+struct D : private C {}; // { dg-message "declared" }
struct X: A, B1, B2, D
{
diff --git a/gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C b/gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C
index 3dfaf7fd0ea..8d6c6f10806 100644
--- a/gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C
+++ b/gcc/testsuite/g++.old-deja/g++.brendan/visibility6.C
@@ -3,9 +3,9 @@
class bottom
{
public:
- int b; // { dg-message "" } private
+ int b;
};
-class middle : private bottom
+class middle : private bottom // { dg-message "" } private
{
public:
void foo () { b; }
diff --git a/gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C b/gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C
index 3c443afe678..c165b0874a0 100644
--- a/gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C
+++ b/gcc/testsuite/g++.old-deja/g++.brendan/visibility8.C
@@ -5,9 +5,9 @@
class foo
{
public:
- static int y; // { dg-message "" } private
+ static int y;
};
-class foo1 : private foo
+class foo1 : private foo // { dg-message "" } private
{ };
class foo2 : public foo1
{ public:
diff --git a/gcc/testsuite/g++.old-deja/g++.jason/access8.C b/gcc/testsuite/g++.old-deja/g++.jason/access8.C
index 4404d8ad95d..0aa85d0457c 100644
--- a/gcc/testsuite/g++.old-deja/g++.jason/access8.C
+++ b/gcc/testsuite/g++.old-deja/g++.jason/access8.C
@@ -3,13 +3,14 @@
// Date: 25 Jan 1994 23:41:33 -0500
// Bug: g++ forgets access decls after the definition.
-class inh { // { dg-message "" } inaccessible
+class inh {
int a;
protected:
void myf(int);
};
-class mel : private inh {
+class mel : private inh // { dg-message "" } inaccessible
+{
protected:
int t;
inh::myf; // { dg-warning "deprecated" }
diff --git a/gcc/testsuite/g++.old-deja/g++.law/access4.C b/gcc/testsuite/g++.old-deja/g++.law/access4.C
index 54072ce3b32..57fa24a0dde 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/access4.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/access4.C
@@ -6,12 +6,13 @@
// Subject: g++ 2.5.5 doesn't warn about inaccessible virtual base ctor
// Message-ID: <9403030024.AA04534@ses.com>
-class ForceLeafSterile { // { dg-message "" }
+class ForceLeafSterile {
friend class Sterile;
ForceLeafSterile() {} // { dg-message "" }
};
-class Sterile : private virtual ForceLeafSterile {
+class Sterile : private virtual ForceLeafSterile // { dg-message "" }
+{
public:
Sterile() {}
Sterile(const char* /*blah*/) {}
diff --git a/gcc/testsuite/g++.old-deja/g++.law/visibility12.C b/gcc/testsuite/g++.old-deja/g++.law/visibility12.C
index 59467ba7d80..6b7ff75d2ca 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/visibility12.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/visibility12.C
@@ -6,11 +6,12 @@
// Subject: member access rule bug
// Message-ID: <9306300528.AA17185@coda.mel.dit.CSIRO.AU>
struct a {
- int aa; // { dg-message "" } private
+ int aa;
};
-class b : private a {
- };
+class b : private a // { dg-message "" } private
+{
+};
class c : public b {
int xx(void) { return (aa); } // aa should be invisible// { dg-error "" } .*
diff --git a/gcc/testsuite/g++.old-deja/g++.law/visibility4.C b/gcc/testsuite/g++.old-deja/g++.law/visibility4.C
index 1cdec1c2b55..644154e66fb 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/visibility4.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/visibility4.C
@@ -8,10 +8,11 @@
class A {
public:
- int b; // { dg-message "" } private
+ int b;
};
-class C : private A { // NOTE WELL. private, not public
+class C : private A // { dg-message "" } private
+{
public:
int d;
};
diff --git a/gcc/testsuite/g++.old-deja/g++.law/visibility8.C b/gcc/testsuite/g++.old-deja/g++.law/visibility8.C
index 5242dfc804f..4457ddf46c7 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/visibility8.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/visibility8.C
@@ -7,11 +7,12 @@
// Message-ID: <m0nof3E-0021ifC@jts.com
class t1 {
protected:
- int a; // { dg-message "" } protected
+ int a;
};
-class t2 : private t1 {
+class t2 : private t1 // { dg-message "" } protected
+{
public:
int b;
};
diff --git a/gcc/testsuite/g++.old-deja/g++.other/access4.C b/gcc/testsuite/g++.old-deja/g++.other/access4.C
index d3c8d85c3f5..6c47700db19 100644
--- a/gcc/testsuite/g++.old-deja/g++.other/access4.C
+++ b/gcc/testsuite/g++.old-deja/g++.other/access4.C
@@ -1,10 +1,10 @@
// { dg-do assemble }
-struct A { // { dg-message "" } inaccessible
+struct A {
static int i;
};
-struct B : private A { };
+struct B : private A { }; // { dg-message "" } inaccessible
struct C : public B {
int f () { return A::i; } // { dg-error "" } context
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-01-22 22:36 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-05 14:24 [PATCH] c++: private inheritance access diagnostics fix [PR17314] Anthony Sharp
2021-01-07 21:01 ` Jason Merrill
2021-01-09 0:38 ` Anthony Sharp
2021-01-11 20:03 ` Jason Merrill
2021-01-21 19:28 ` Anthony Sharp
2021-01-21 22:56 ` Jason Merrill
2021-01-22 3:44 ` Jason Merrill
2021-01-22 20:07 ` Anthony Sharp
2021-01-22 21:18 ` Jason Merrill
2021-01-22 22:36 ` Anthony Sharp
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).