Ambiguity in forwarded vs inherited methods.

[ian-bertolacci]

class Base {
  proc useImplement() {
     return mustImplement();
  }
  proc mustImplement() : int {
    halt("Derived classes must implement this method");
    return -1;
  }
}

class DirectlyDerived : Base {
  proc mustImplement() : int return 1234;
}

class MultiDerived : Base {
  var derived : DirectlyDerived;
  forwarding derived;

  proc init(){
    this.derived = new DirectlyDerived();
  }
}

writeln( (new DirectlyDerived()).useImplement() );
writeln( (new MultiDerived()).useImplement() );

In the above code, Both DirectlyDerived and MultiDerived inherit from Base, which defines a "virtual method" mustImplement. However, MultiDerived also has a DirectlyDerived member variable which it forwards; it does not implement mustImplement.

This compiles fine, but invoking MultiDerived.useImplement, in turn invokes Base.mustImplement instead of DirectlyDerived.mustImplement (causing a halt) which may be unexpected.

The obvious work-around is to define MultiDerived.mustImplement and have it call its member's DirectlyDerived.mustImplement method. But this negates (and in this case, entirely negates) the purpose of using forwarding.

While it makes sense that subclass-inherited methods take precedence over forward-inherited methods, it would be nice if there was (1) a warning when this happened, and (2) a way to override this behavior (something like forwarding derived override mustImplement).

[ian-bertolacci]

A preview real-world example is here in #10427.
Because ArrayViewRankChangeDom inherits from BaseRectangularDom, but forwards a member DefaultRectangularDomain, I have to define dsiDim to call the member domain, since dsiDim is abstract in BaseRectangularDomain. Avoiding these kinds of redefinitions is the spirit of both this refactor and forwarding.

[bradcray]

@mppf: As the person who's spent the most time with forwarding, did you have thoughts about this one? Seems like this is a more interesting question if we imagine having pure virtual functions (where I'm imagining that mustImplement() would become one).

[mppf]

This is very similar to #7786.

If we had pure virtual methods, I think it'd be "obvious" that they should be "last resort" and that, as a result, the forwarding method should take precedence over them. That would require some implementation adjustment though (right now, forwarded methods never take precedence over "last resort" methods... but I don't think it'd be so hard check for non-last-resort forwarded methods if there is a "last resort" method.).

Do we plan to actually add pure virtual methods any time soon?

[bradcray]

Do we plan to actually add pure virtual methods any time soon?

I'd like for us to. I think that there have been lots of recent arguments in favor of them, and few (none?) in opposition. Am I remembering correctly that we thought they could help with some of the hijacking cases you walked us through earlier this year?

[mppf]

@bradcray - yes, it could help in one case - https://github.com/chapel-lang/chapel/blob/master/doc/rst/developer/chips/20.rst#unexpected-new-overload-for-an-overridden-method - but that case could also be helped by other means (specific errors for certain combinations of override+overload).

Anyway I think we have enough methods with a body of "halt" that are meant to be pure virtual that we should do it. Not sure it has an issue yet.

[mppf]

thanks to @ronawho for pointing out - we have issue #8566 for pure virtual methods.