Bug 17314 - Error message wrongly shows declared rather than inherited access
Summary: Error message wrongly shows declared rather than inherited access
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 3.4.0
: P2 normal
Target Milestone: 11.0
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2004-09-04 04:00 UTC by Ivan Godard
Modified: 2021-01-29 13:03 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-12-25 04:59:02


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ivan Godard 2004-09-04 04:00:22 UTC
class   A {
protected:
        A(){}
        };
class B : virtual A {
    };
class C : public B {};
int main () {
    C c;
    }

gets you:

~/ootbc/common/test/src$ c++ foo.cc
foo.cc: In constructor `C::C()':
foo.cc:3: error: `A::A()' is protected
foo.cc:9: error: within this context

The message should say "`A::A()' is hidden by virtual inheritance"; "protected"
makes no sense seeing as how we are trying to reference it from a derived
class. 

Comeau gives:
"ComeauTest.c", line 7: error: "A::A()" is inaccessible
  class C : public B {};
                   ^
          detected during implicit generation of "C::C()" at line 9

1 error detected in the compilation of "ComeauTest.c".

which is marginally better but still not great. BTW, why virtual base classes
block visibility beats me, but that seems to be the standard.

Ivan
Comment 1 Kriang Lerdsuwanakij 2004-09-04 12:13:25 UTC
Not a bug.  Mentioning the base class as simply 'virtual A'
means 'private virtual A'.  Perhaps you want the following code
which compiles on GCC:

class   A {
protected:
        A(){}
        };
class B : public virtual A {
    };
class C : public B {};
int main () {
    C c;
    }

Comment 2 Ivan Godard 2004-09-04 19:32:25 UTC
Please read the original report again. I knew that this is an error according to the standard; the complaint is about the error message used to report it, which is confusing and makes no sense. By definition a derived class can reference protected base names, so a message that the name can't be referenced because it is protected makes no sense. The name is in fact private as you point out - but that's not what the message says. This is a quality-of-implementation issue, not a correctness issue.

Ivan
Comment 3 Kriang Lerdsuwanakij 2004-09-06 15:58:16 UTC
What about an error message that says "error: `A::A()' is private"?
Comment 4 Ivan Godard 2004-09-06 21:37:21 UTC
Sure, although if convenient and the message is used only here then it might be
helpful to say why it's private: "virtual base class defaults A::A() to
private" or similar; only saying it's protected that is clearly wrong. For sure
the compiler is detecting that the *inherited* visibility is private to trigger
the error, but the message is then reporting the *declared* visibility, which is
wrong.

In the more common case:

struct A { void Foo(){} };
struct B: private A {};
int main () {
    B b;
    b.Foo();
    }

you get:

~/ootbc/common/test/src$ g++ foo.cc
foo.cc: In function `int main()':
foo.cc:1: error: `void A::Foo()' is inaccessible
foo.cc:5: error: within this context
foo.cc:5: error: `A' is not an accessible base of `B'


which is very good - can't my case plug into the logic that produces this?

Ivan

you get:


Ivan
Comment 5 Andrew Pinski 2004-09-07 07:49:03 UTC
Still the error that GCC gives is about the same as what Comeau gives so again this is not a bug. 
Also it does explain what is wrong A::A() is not accessible.
Comment 6 Ivan Godard 2004-09-07 08:15:25 UTC
One last try: Comeau says that it is inaccessible, which while not all that 
informative is correct as far as it goes. gcc says that it is "protected", which is *actively misleading*; the error is reported for a reference in a derived class which by the definition of "protected" should be able to reference any protected name in a base class.

Bottom line: I spent half a day on a wild goose chase prompted by this message, which sent me off in completely the wrong direction from the actual problem. I didn't really catch on until I ran a reduced case past Comeau. Not everyone is intimately familiar with the visibility semantics of virtual base classes (they are a rarely used feature) and I found no mention of the difference in default visibility between virtual and non-virtual base inheritance in several popular reference books, D&E, and The C++ Language (Stroustrup).

If you are producing a compiler intended solely for language mavens like yourselves, then leave this message; it does report an error after all. But if you want your compiler to be used by real people, and perhaps more importantly if you want your compiler to encourage people to actually use the C++ language as opposed to run C through a C++ compiler, then this message is way misleading and should be changed.

</FLAME>  I won't reopen again.

Ivan
Comment 7 Andrew Pinski 2004-09-07 08:32:22 UTC
But Comeau does not say why it is inaccessible while we do, it is protected. 
Comment 8 Ivan Godard 2004-09-07 11:34:04 UTC
You still don't get it, so I give up. You might want to run this by someone else at gnu; maybe they can explain. 

"But Comeau does not say why it is inaccessible while we do, it is protected." FALSE. It is NOT inaccessible because it is protected - any derived class can access a protected member in a base class. It is inaccessible because it is in fact PRIVATE - which is not what the message says.

Have fun. I don't know why I waste my time trying to improve your product.

Ivan
Comment 9 Gabriel Dos Reis 2004-09-07 12:34:45 UTC
Technically, we gave a diagnostic.  But, from a QoI we can improve 
on this.
Comment 10 Gabriel Dos Reis 2004-09-07 12:35:38 UTC
assigned to self
Comment 11 Ivan Godard 2004-09-07 13:14:48 UTC
Thank you :-)

Ivan
Comment 12 Jason Merrill 2006-03-21 02:48:55 UTC
tweak summary
Comment 13 Paolo Carlini 2013-05-16 12:49:00 UTC
I may be missing details, but I'm not sure we have a serious problem here. Note that:

class   A {
public:
        A(){}
        };
class B : virtual A {
    };
class C : public B {};
int main () {
    C c;
    }

is fine. Thus, to be clear, isn't that the "declared access" doesn't matter. The real issue seems that the complete rules of protected access are complex and hard to, so to speak, summarize in the error message. Gaby?
Comment 14 Paolo Carlini 2013-05-16 13:00:06 UTC
If I understand correctly the issue, saying in enforce_access something like "A::A(), declared protected, is inaccessible within this context" instead of "A::A() is protected within this context" would be less confusing.
Comment 15 Jason Merrill 2013-05-16 14:07:51 UTC
Yes, that would be an improvement to the diagnostic.  But it seems to me that there's a deeper issue here: I think both testcases should be ill-formed because C::C can't form a pointer to its A base in order to try to call its constructor.  Just as in this testcase:

class A {
protected:
  void g();
};
class B : virtual A { };
class C : public B {
  void f() { ::A::g(); }
};
int main () {
  C c;
}

Here if we could convert 'this' to an A*, we would be OK.  Now, the magic conversion for vbase construction is special, and the standard doesn't really say how special; if it has special access (to go along with its special ability to choose a specific subobject in a hierarchy that might have multiple bases of the same type), the original testcase should be ok: we're calling a protected member through a C object, which is fine by the rules for protected access.  If the constructor doesn't have special access, then it's ill-formed even if the constructor is public.

I think a core issue is warranted.  But for the mean time, making that change to the diagnostic would be an improvement.
Comment 16 Paolo Carlini 2013-05-16 14:51:56 UTC
I see, thanks Jason. Indeed, the behavior of various compilers I have here is inconsistent about the various variants of the testcase.

Thus for now I'm going to test the diagnostic tweak with the plan of committing something and then immediately suspending the PR. If/when you have a DR # we can point to it in the Subject.
Comment 17 Jason Merrill 2013-05-16 20:08:48 UTC
Actually, this seems to be issue 7:

  http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_closed.html#7

So we should reject the variant with a public constructor as well.  It would be good to get the same diagnostic as in comment #4.
Comment 18 Jason Merrill 2013-05-16 20:09:48 UTC
(In reply to Jason Merrill from comment #17)
> It would be good to get the same diagnostic as in comment #4.

And also give a warning about the private virtual base.
Comment 19 Paolo Carlini 2013-05-16 23:49:17 UTC
I see, thanks.
Comment 20 kamilek1313 2019-11-01 22:30:45 UTC
I think the point of this issue was missed. It has nothing to do with constructors/destructors, it can be reproduced with the following example:

class Grandparent {
protected:
    int foo = 0;
};

struct Parent : private Grandparent {
};

struct Child : public Parent {
    void bar() {foo = 1;}
};

Currently (GCC 9.2.0, also present in experimetal), the following error is issued:

prog.cc: In member function 'void Child::bar()':
prog.cc:10:17: error: 'int Grandparent::foo' is protected within this context
   10 |     void bar() {foo = 1;}
      |                 ^~~
prog.cc:3:9: note: declared protected here
    3 |     int foo = 0;
      |         ^~~


See Wandbox: https://wandbox.org/permlink/mwF205Xm6A3xcKGy 

This error message is just wrong. Grandparent::foo is private within this context, due to private inheritance. If it was protected, it would be accessible by Child. Upon seeing this message, I have no clue what is wrong (yeah, it is protected, so what?)

For comparison, clang 10 produces clear(ish) error message for the same code:

prog.cc:10:17: error: cannot cast 'Child' to its private base class 'Grandparent'
    void bar() {foo = 1;}
                ^
prog.cc:6:17: note: declared private here
struct Parent : private Grandparent {
                ^~~~~~~~~~~~~~~~~~~
prog.cc:10:17: error: 'foo' is a private member of 'Grandparent'
    void bar() {foo = 1;}
                ^
prog.cc:6:17: note: constrained by private inheritance here
struct Parent : private Grandparent {
                ^~~~~~~~~~~~~~~~~~~
prog.cc:3:9: note: member is declared here
    int foo = 0;
        ^
2 errors generated.
Comment 21 Igel 2020-04-10 15:22:24 UTC
Seconded! This bug still exists in 9.2.0. Here's another example:

class Base {  protected: int i; };
class Derived:  public Base    { using Base::i; };
class Derived2: public Derived { using Derived::i; };
int main(){}

> g++ test.cpp -o test
test.cpp:3:7: error: 'int Base::i' is protected within this context
    3 | class Derived2: public Derived { using Derived::i; };
      |       ^~~~~~~~
test.cpp:1:30: note: declared protected here
    1 | class Base {  protected: int i; };
      |                              ^

no mention of line 2 where the actual problem is (namely that i is declared private (not protected)).

cheers
Comment 22 Anthony Sharp 2020-12-29 00:14:51 UTC
I am currently working on a fix for this.

There seems to be three issues here. 

1. The primary issue is that in cases of private inheritance, GCC incorrectly reports that the member is inaccessible because it is protected, when the (more correct) reason is because it is private. This has required some re-working of the diagnostics code.

2. The second issue (more of a discussion topic) seems to revolve around whether virtual inheritance is private by default. A few people here have said that it is. They might be right, but I am not sure.

3. The third issue seems to be an academic debate on the proper effect of a virtual private inheritance. Currently, it is possible to inherit from a private virtual base, e.g. the following code compiles:

class Foo { public: Foo(){}  ~Foo(){} };
class A:virtual private Foo { public: A(){}  ~A(){} };
class Bar:public A { public: Bar(){}  ~Bar(){} };

As Jason Merrill points out, this wasn't intended (http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_closed.html#7). Also see https://stackoverflow.com/questions/3900192/msvc9-0-bug-or-misunderstanding-of-virtual-inheritance-and-friends/3917482. Apparently MSVC9 has the same quirk.

I'm not sure why the C++ standards committee would say it was intended for the above code to be ill-formed (or that's what they seem to say). There seems to me to be no access violation in the above code, technically speaking. Jason Merrill says '~Bar() calls ~Foo(), which is ill-formed due to access violation, right? (Bar's constructor has the same problem since it needs to call Foo's constructor.)', but as far as I'm aware, constructors and destructors aren't affected by access protection levels. It is possible for a class to derive from a private class (e.g. see https://stackoverflow.com/questions/30572835/c-private-class-inheritance). Jason Merrill points out that there might be some under-the-hood issues with the above code, and he might be right, but that's beyond my current level of understanding.

Even if such code is ill-formed, it doesn't really bother me (and nor has it bothered anyone else, considering this bug report was issued 16 years ago). Virtual private inheritance is a feature that virtually (haha, get it?) no one uses, and I think it'd be a waste of time to patch a dead feature. So, I intend to only fix issue 1. After that, I might create a new bug report to do with 3., just so the issue is not buried.

Any thoughts are welcome.
Comment 23 Anthony Sharp 2021-01-06 13:11:48 UTC
The patch is now on the mailing list (https://gcc.gnu.org/pipermail/gcc-patches/2021-January/562835.html).

Please take my last comment with a pinch of salt ... I was mainly trying to sum up what has already been said. There are plenty of people here who have much more knowledge than I do and if they say that something is the case, it probably is.

Also just to be clear when I said that no-one uses virtual private inheritance, that obviously isn't strictly true. What I meant to say was that no one would realistically use a virtual private inheritance, and then try to derive another class from it, which is what the problem is.
Comment 24 Ivan Godard 2021-01-06 23:26:37 UTC
OP here. 

Yes, no one would *intentionally* try to derive from a virtual private. But one could - and I did - derive a class with. It took no little wild geese chasing from the diagnostic to find that a virtual private was the real, and unreported, problem.

So I filed a *UI* and *QOI* ticket. Compilers exist not only to translate correct code, but also to help with the inevitable incorrect code. After all, more of the latter gets submitted for compilation.

To say that a code is nonsense and so can be ignored is a disservice to the community that gcc purports to serve.

In fairness, gcc and its maintainers are much less arrogant these days than when this ticket was filed. But I notice that bugzilla shows *twenty seven* tickets filed by me and still open. Every one of those is a dozen years old or more, because around then I got fed up with the gcc attitude, switched to clang, and have never been back.

Except for the occasional rant like this.
Comment 25 Anthony Sharp 2021-01-07 00:57:47 UTC
Hopefully (pending approval) the original bug is now fixed, even if it did take a long time! I agree that compilers should match the standard where reasonable to do so (like in this case), but I can't comment much specifically on the issue of whether deriving from a virtual private inheritance should be ill-formed (it seems the general consensus is that it should be, although one can find people debating the opposite on various forums e.g. https://stackoverflow.com/questions/2371176/c-private-virtual-inheritance-problem). Although I could probably go and try find out, I think it'd be best to leave that to those who know a bit more about the topic than me; this is only my first ever patch submission after all :)
Comment 26 Anthony Sharp 2021-01-29 12:34:13 UTC
Fixed. It seems the issue to do with the virtual class constructors is a duplicate of 55120 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55120).
Comment 27 Jonathan Wakely 2021-01-29 13:03:21 UTC
Fixed by r11-6880, thanks!