3

My question is about why the following code behaves as it does. I'm not asking about the quality of the design (I know some of you will immediately hate the multiple inheritance and I'm not arguing for or against it here) I could ask a separate question that goes into what the author was trying to achieve, but lets just assume there is code that is equivalent to this:-

class IReadableData
{
    public: virtual int getX() const = 0;
};

class Data : public virtual IReadableData
{
   public: 
      virtual int getX() const { return m_x; }
      void setX(int x) {m_x = x;}
   private:
      int m_x;   
};

class ReadableData : private Data, public virtual IReadableData
{
     // I'd expected to need a using here to expose Data::getX
};

This complies on visual studio 2017 with "warning C4250: 'ReadableData': inherits 'Data::Data::getX' via dominance"

Firstly I was a bit surprised not to be told that ReadableData didn't implement getX (given its implementation is private), but no warning occurs and I can create a ReadableData, but although ReadableData publicly inherits from IReadableData, the public methods of IReadableData are inaccessible

    ReadableData r;
    //  r.getX(); error C2247: 'Data::getX' not accessible because 'ReadableData' uses 'private' to inherit from 'Data'

However the following does compile

    ReadableData r;
    IReadableData& r2 = r;
    r2.getX();

This seems inconsistent to me either r is an IReadableData and getX should be available or it isn't and the assignment to r2 (or the definition of ReadableData) should fail. Should this happen? what in the standard leads to this?

This question:- Diamond inheritance with mixed inheritance modifers (protected / private / public)

seems connected, except that the base in that case is not abstract, and its answer citing section 11.6 would make me think that r.getX(); should have been accessible.

Edit: I made example code slightly less minimal to give a tiny bit of insight into intention, but it doesn't change the question really.

ROX
  • 1,256
  • 8
  • 18
  • I'm confused... You're trying to say something like "ReadableData has the interface IReadableData, but implementation of Data"? That's just Data and you typedef it. – UKMonkey Oct 27 '17 at 14:33
  • @UKMonkey its a minimal example, its not real code. – ROX Oct 27 '17 at 14:35
  • @Ron, "avoid multiple inheritance"? What even of interfaces? even C# allows that (although it wouldn't have allowed the private inheritance). – ROX Oct 27 '17 at 14:45
  • The usual way to do this is to have a private member of Data, and then the impl of ReadableData just wraps the Data functions. The problem with inheritence isn't the multiple part, but the fact that they do it when they shouldn't. – UKMonkey Oct 27 '17 at 15:27
  • @UKMonkey, thanks, yes I agree and that's how I would usually do this myself. I'd still like to understand why code like this behaves as it does though. – ROX Oct 27 '17 at 15:33

2 Answers2

3

It's a derivative of the lookup rules, which have some virtual inheritance related bullet points. The idea is that virtual inheritance should not cause ambiguity during name lookup. So a different declaration of getX() is found in each scenario. Since access specifiers are only checked after name lookup (and do not affect name resolution) you can hit such snags.

  • In the case of r2.getX();, lookup begins in the context of IReadableData. Since a declaration is found immediately, and IReadableData has no bases, this is what the lookup resolves to. It's a public member of IReadableData, so you can name and call it. After that, it's the dynamic dispatch mechanism that takes care of calling the implementation given by the dominating Data::getX().

  • In the case of r.getX();, lookup works differently. It starts in the context of ReadableData. There is no declaration of getX() present, so it turns to look at the immediate bases of ReadableData. And here things get a bit un-intuitive:

    1. It checks IReadableData, and finds IReadableData::getX(). It take note of this lookup, and the IReadableData base sub-object it was found in.
    2. It checks Data, and finds Data::getX(). A note of this lookup, and the Data base sub-object it was found in is also taken note of.
    3. Now it tries to merge the lookup sets of #1 and #2 into the lookup set of ReadableData. Since the IReadableData sub-object in #1, is also a sub-object of the Data sub-object from #2 (on account of virtual inheritance), all of #1 is completely ignored.
    4. Being left with only Data::getX() in step #3, the lookup resolves to it.


    So r.getX(); is in fact r.Data::getX();. That's what lookup finds. And it is at this point that access specifiers are checked. Which is the cause of your error.

Everything I said is an attempt to break down the process described in the standard by the section [class.member.lookup]. I didn't want to quote the standard on this, because I feel it doesn't go a lot towards explaining what happened in plain English. But you can follow the link to read the full specification.

StoryTeller - Unslander Monica
  • 165,132
  • 21
  • 377
  • 458
  • Thanks, I'd suspected it was to do with the lookup rules, a quick look at the standard shows you're probably right not to quote it directly - but this is what I was after really when I mentioned the standard, an answer that was based on it. – ROX Oct 30 '17 at 10:50
1

Access modifiers are always resolved statically, not dynamically, i.e. it is based on the static and not dynamic type. The contract of an object implementing IReadableData, is strictly speaking that a pointer/reference to that object can be aliased as an IReadableData, and then a method getX can be called meaningfully on it. After all, the whole point of polymorphic contracts is to use them, polymorphically. There's no real guarantee, or necessity of one, on what happens when you use the derived object directly.

So, in that sense, allowing derived objects to change access specifiers, but then then resolving the access specifiers based on the static and not dynamic type, is at least a choice that is consistent with the notion of a polymorphic contract.

All that said, changing access specifiers in derived objects is not really a good idea in any way. There's no upside because it's trivial to work around, so there's zero encapsulation benefit, it just exposes this bizarre edge case.

Design wise I am not fundamentally against multiple inheritance. However, diamonds are something you are better off avoiding, 99% of the time. Private inheritance as well, has almost no uses, and they are simpler to enumerate:

  1. For the empty base class optimization.
  2. If someone else has written a class that has virtual functions as a technique to customize it, and you need this class to implement yours. I say "someone else" because this design in more modern C++ is hard to justify, now that it's easier to just pass around functions/lambda/std::function.
Nir Friedman
  • 17,108
  • 2
  • 44
  • 72
  • 1
    But then why doesn’t `r.getX();` work? It might not break polymorphism, but I don’t see why it shouldn’t be public in `ReadableData`. – Daniel H Oct 27 '17 at 17:20
  • @DanielH I'm not sure if I understand the question. Mechanistically it's like I already said: access modifiers are resolved statically. If you think about it, purely from a performance point of view, it would be crazy to check access specifiers at *runtime*. There's almost no upside. Aside from checking the dynamic type, the only other alternative the language has here is to disallow you from changing access specifier in the derived class. Why didn't C++ do this? Usually C++ opts for flexibility, and heavier OOP used to be fashionable, where perhaps it might help more. Nowadays, not so much. – Nir Friedman Oct 27 '17 at 20:02
  • Yes, they are resolved statically, which explains why `ReadableData::getX` and `IReadableData::getX` *can* have different access modifiers. But why, in this particular example, *do* they have different access modifiers. What determines that `ReadableData::getX` is `private` instead of `public` in this case? – Daniel H Oct 27 '17 at 20:07
  • @DanielH The implementation comes from `Data`, and `Data` is inherited from privately. When you inherit from something, all of data/methods access are capped at the inheritance level. So if you inherit privately from `Data`, everything you inherited from `Data` is now private. The actual `getX` function comes from `Data`, not from `IReadableData`. Does that make sense? Or maybe that last point is the issue; why is the access level based on `Data` and not `IReadableData`? – Nir Friedman Oct 27 '17 at 20:53
  • Yes, that’s the question I was asking. Why does the access level come from `Data` and not `IReadableData`? Your comment and StoryTeller’s answer seem to say it’s because the implementation comes from `Data`; is that a fair rule of thumb? – Daniel H Oct 27 '17 at 22:30