chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.79k stars 420 forks source link

Support operator methods and inheritance #17097

Open lydia-duncan opened 3 years ago

lydia-duncan commented 3 years ago

Summary of Problem

Got initial implementation of operator methods off the ground, but inheritance is slightly more tricky. Current behavior is a bunch of messages about coercion. Shouldn't be too difficult to fix, but I didn't want to remove the test I'd already committed.

Steps to Reproduce

Source Code:

class Parent {
  var x: int;

  operator <(lhs: Parent, rhs: Parent) {
    return lhs.x < rhs.x;
  }
}

class Child: Parent {
  var y: bool;
}

var c1 = new Parent(2);
var c2 = new Child(3, false);
writeln(c1 < c2);
writeln(c2 < c1);
var c3 = new Child(4, true);
writeln(c2 < c3);
writeln(c3 < c2);

Compile command: chpl foo.chpl

Associated Future Test(s): test/functions/operatorOverloads/operatorMethods/inheritance.chpl #17103

Configuration Information

bradcray commented 3 years ago

I don't think our intention is for operators to be inherited. Like the current standalone functions and the previous 'type method' approach, they should only dispatch based on static types. Inheritance would result in too many problems w.r.t. multiple dispatch, I believe. So maybe you're closer than you think?

[edit: I was thinking about inheritance in a different way than I think you meant in this comment and missed that your example only was dispatching based on static types. My next comment is more on-topic].

bradcray commented 3 years ago

I need to amend that previous statement having spent more time thinking about this and studying the code example. I don't think they should be inherited in the way a (value) method is since we want them to be similar to a type method or the standalone functions we currently have; but I do agree that if they are visible because the methods for the type on which they are defined are visible (as in this example) then they should be callable if they would be for a standalone case that's visible (e.g., child class actuals should be able to be passed to parent class formals, as usual, and as in your example). So I agree this example should work (but a little surprised that it would take any special effort, since I'd think it would just fall out through normal resolution / coercion rules).

This raises for me the following visibility question:

module M {
  class C {
    var x: int;
    proc type foo() { writeln("In C.foo()"); }
    operator +(c1: C, c2: C) { return new C(c1.x + c2.x); }
  }
}

module N {
  import M.C;
  class D: C { }
}

module Main {
  import N.D;
  var myD = new D();
  D.foo();  // this shouldn't resolve I assume, either because C.foo() isn't visible or because 'type D' is not 'type C'
  var result = myD + myD;   // should this?  Can I see C.+ given that I've only imported 'D'?
}

This is the sort of thing I was thinking should be illegal ("inheritance doesn't apply here") in my original comment. But now I'm feeling less sure since these operators arguably straddle the boundary between type methods (which wouldn't resolve in a case like this) and value methods (in that the only arguments passed to them are values, unlike normal type methods).

lydia-duncan commented 3 years ago

myD.foo(); // this shouldn't resolve I assume, either because C.foo() isn't visible or because 'type D' is not 'type C'

I think this shouldn't resolve because it isn't written correctly. As a type method, shouldn't it be called via C.foo()? Or, to make a closer comparison, D.foo()?

When resolving normal methods, we jump to the scope where parent types are defined as well as the scope where child types are defined (this was mentioned at least in passing as part of this comment, but I can see missing it and paying attention to other parts of the discussion). I would expect us to fail on C.foo() because C is not a symbol we can resolve in the scope of Main, but I would expect D.foo() to behave the same as if all the types were defined in the same scope.

bradcray commented 3 years ago

Good point, I did mean D.foo(); and will edit it to reflect that. The real question I was trying to ask here was supposed to be the next line, though: Should myD + myD resolve?

lydia-duncan commented 3 years ago

I think it should. Or rather, I think it should behave the same as D.foo() in that scope. But I think it's interesting to point out - if you remove the + line in the code you've written, that code behaves differently than if all the types were in the same scope. E.g. this code compiles and runs without issue:

module M {
  class C {
    var x: int;
    proc type foo() { writeln("In C.foo()"); }
    operator +(c1: C, c2: C) { return new C(c1.x + c2.x); }
  }

  class D: C { }

  var myD = new D();
  D.foo();
}

But this code does not:

module M {
  class C {
    var x: int;
    proc type foo() { writeln("In C.foo()"); }
    operator +(c1: C, c2: C) { return new C(c1.x + c2.x); }
  }
}

module N {
  import M.C;
  class D: C { }

  var myD = new D();
  D.foo();
}

That seems like a bug with importing type methods to me (oops), but the first code also seems different than what you were asserting earlier about type methods and inheritance.

I think it would also be reasonable to say that what I'm pointing out occurs in this comment is incorrect behavior and we should only allow type methods to be called on the exact type on which they were defined except in the case of operators. I just personally think both should work.

lydia-duncan commented 3 years ago

Through experimenting and talking with Michael, this issue is more closely tied to #8489 and for now the recommended strategy is to define operators on classes to take the borrowed version of the type in case it gets inherited from. I'm going to change my intended action to documenting this, mentioning it on #8489 and leaving this issue open on its own.