Chapel: override checking/virtual dispatch and default arguments

[mppf]

What should the combination of overriding and default arguments actually do? What if the default arguments are different, as in this example? What if they are the same?

proc f() {
  return 0;
}

proc g() {
  return 1;
}

class Parent {
  proc method(arg:int = f()) {
    writeln("in Parent.method arg=", arg);
  }
}

class Child : Parent {
  override proc method(arg:int = g()) {
    writeln("in Child.method arg=", arg);
  }
}

proc main() {
  var x:Parent = new Child();
  x.method();
  // should the above call:
  //   1. be in error (or maybe the error is elsewhere in the program)
  //   2. run child's default (virtually dispatched) `g()` and so print out `in Child.method arg=1`
  //   3. run parent's default `f()` and so print out `in Child.method arg=0`

  var y = new Child();
  y.method(); // expect this to run `g` and so print `in Child.method arg=1`
}

I can think of the following options for how this is handled:

1. It is an error
   1a. It is an error to override any proc with a default argument
   1b. It is an error only if the default argument expression is not identical.
   
   
   
   1. Default arguments in overridden functions use dynamic type / do virtual dispatch. That would result in running the child's default argument expression g() in the x.method() call in the example.
   
   
   2. Default arguments in overridden functions ignore dynamic type and are never subject to virtual dispatch. That would result in running the parent's default argument expression f() in the x.method() call in the example.
   
   

On master, this results in a compilation failure under --devel but not otherwise, whether the default arguments are the same expression or no. If it compiles, it uses option 2 (and so prints out 1 in both cases). Additionally, the behavior changes for param vs non-param default expressions (see #14167).

See also #7880 which asks more or less the same question.

[mppf]

@e-kayrakli - this issue asks the design question related to the problem you were running into.

[e-kayrakli]

Thanks for creating the issue @mppf!

Just to record my reflex: I expect the behavior to be 2. i.e. if there is no default argument, then x.method() calls the child's override. Why would having a default argument change that behavior?

[mppf]

@e-kayrakli - to be clear, either way x.method() calls Child.method(). The question is - which default argument expression for arg does it run? Does it run f() or g()?

[e-kayrakli]

@mppf -- right. I think g() should be called. To me, Child.method()'s default argument is g() regardless of whatever we have for the parent.

Also, my initial thoughts on those options:
1a. Not a big fan.
1b. I can learn to live with it

1. I like
2. I don't like

[bradcray]

My intuition here is also that g() should be called. It seems to me that the presence/absence of default values should probably be a consideration in override checking, but that the specific default values should not be.

Note that the options listed in the comments in the code are in a different order than the options listed in the issue itself, which could lead to confusion.

[mppf]

Note that the options listed in the comments in the code are in a different order than the options listed in the issue itself, which could lead to confusion.

Oh, whoops. I fixed the code comments to match. Thanks for pointing it out.

[cassella]

If it compiles, it uses option 2 (and so prints out 1 in both cases).

I don't see the failure with --devel, but I do see the "Child: 1" behavior for both. Curiously, the code in #7880 (with an added override) still prints different things for its corresponding (and nearly identical) cases. The difference is just that in that issue, the default values are given as int literals, instead of calls to functions that return ints.

FWIW, I like option 2 too. My intuition is the default values are somehow internal to the function, as if the first line was "if arg wasn't specified, set it to the default". So entering Child.method() and being handed Parent.method()'s default value (when the caller left it unspecified) is really surprising, since my intuition is that the setting of the parent's default value is something that happens inside a function that wasn't called!

Edit: though I see checking the spec that my intuition is wrong. The default value expression is specified to be evaluated (and thus resolved) in the caller's scope, so it's effectively part of the function's interface.

[mppf]

@cassella - thanks for pointing out #7880 which is more or less a duplicate of this one.

[cassella]

For the record, the test above on master now produces

in Child.method arg=1
in Child.method arg=1

Adding the param tag to f() and g() results in

in Child.method arg=0
in Child.method arg=1

[cassella]

If the default values are considered to be an internal implementation
detail of the function, as my intuition was, then option 2 makes sense
-- a child class overriding method can choose a different internal
implementation detail. This is the behavior of the code in the issue
as written.

If the actual default values (and not just the existence of default
values) are part of the function's interface, as the spec says, then I
think it has to be one of option 1. A child class can't override a
method with a method that has a different interface.

(I think 1a, which I'd phrase as "a method marked override can't
specify default values for any of its args" is reasonable, simple to
explain, simple to enforce (I presume), presents no novel
point-of-instantiation problems, and provides as much functionality as
1b. (edit: I'm assuming a direct invocation of the child method
through a child class would pick up the default values specified in
the parent method. (This maybe comes more from a C++ "virtual
function" outlook than a Chapel "overridden method" outlook.) This
has the drawback that the child method has default values for its
arguments that aren't visible from the source of the child class.))

I think that last parenthetical leads me to understanding option 3 a
little better. The default values given in a class method are part of
the calling convention of calling the method through a variable of
that static type. Calling through a parent class variable gets you
the parent method's default values, no matter what the call is
virtually dispatched to. A child class may give its method whatever
calling conventions it likes, but they're used only when making a call
on a variable whose static type is the child's type. So its method
might not even have default values, let alone be required to inherit
or replicate the parent's method's default values.

[benharsh]

Revisiting this topic as we near the 1.21 release...

It seems most are on board with g() being called. This is implemented on master in the case where the formal is not param and the default expression is not param (as is the case in the motivating example). On master g() is not called when the default expression is param:

proc f() param { return 0; }
proc g() param { return 1; }

I think this is more of a bug than a design issue. The compiler currently decides to fold the compiler-generated default-expression wrapper away before it knows it needs to be virtually dispatched.

The issue is further complicated if the formal itself is param:

proc Parent.method(param arg:int = f()) { ... }
override proc Child.method(param arg:int = g()) { ... }

I think there's probably a way to get this working the way we want by placing Parent.method(0) and Child.method(1) in the same virtual dispatch slot, but I'd need to know more virtual dispatch on param/type things first. For now, I'd advocate for making this an error that we can relax later.

Two questions:

A) Does anyone object to making the param-formal case an error (for now)?

B) Does Paul's latest comment change anyone's mind on the entire topic? I think this snippet could be worth discussing:

If the actual default values (and not just the existence of default
values) are part of the function's interface, as the spec says, then I think it has to be one of option 1.

[mppf]

It seems most are on board with g() being called.

This is what most people seemed to expect, including Paul in #7880 (or at least that's how I read it). It sounds good to me.

Does anyone object to making the param-formal case an error (for now)?

Sure. I actually don't think it'd be that hard to fix, either.

Does Paul's latest comment change anyone's mind on the entire topic?

I think we need to update the spec with our decision here - identical signatures needs to include intent, type and existence of default value - but not the default value itself. Other than that, I am not concerned.

[bradcray]

I agree with Michael's responses.

[bradcray]

And to add something that wasn't obvious to me immediately, and may not be to others: In a case like this:

proc f() param { return 0; }
proc g() param { return 1; }

class C {
  proc foo([param] x =f()) { ... }
}

class D : C {
  override proc foo([param] x = g()) {
}

var myClass: C = if coinFlip() then new C() else new D();
myClass.foo();

I expect that the compiler will evaluate both f() and g() at compile-time to determine both of the potential param values that will be required by the dynamic dispatch, not being sure which will be used.

(It may also be helpful to note that we can't support dynamically dispatched methods that return params because that would require the compiler to know the dynamic dispatch outcome statically in order to make and evaluate the call. This is similar to why it generally isn't useful to put compilerError()s into a dynamically dispatched method... the compiler has to assume that any/all of the overrides can be called).

[benharsh]

The param-related bug cases are now tracked in #15164.