chapel-lang / chapel

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

How should operator methods interact with forwarding? #16992

Open lydia-duncan opened 3 years ago

lydia-duncan commented 3 years ago

Brad brought up on the operator keyword issue that we should think about how operators as methods should interact with forwarding.

record Type1 {
  var x: int;

  proc type +(lhs: Type1, rhs: Type2) { ... }
}

record Type2 {
  forwarding var a: Type1;

}

I can see a couple of stances here.

One stance is that if the forwarded type is so important to the behavior of the type that contains it, it's probably reasonable to assume that the operators defined on the forwarded type would be useful and so should be included.

The other is that it could be surprising to discover that suddenly your type is able to be added or multiplied or compared if you weren't expecting it, when you were focusing on a specific other aspect of the forwarded type's functionality. Especially when the forwarded field is generic - suddenly sometimes your type can get added and sometimes it can't.

Another thing to note is that operators (and type methods in general) can have different behaviors depending on the operator - > and <, e.g., are expected to return a boolean so can reasonably have their behavior used, but + and - are frequently expected to return a new instance of the types being combined. Having the addition operation on instances of Type2 in the example above return a Type1 would probably be surprising for the person trying to add two instances of Type2.

A savvy user can write an explicit version of the operator for Type2 to avoid this problem. They could also limit the forwarded methods from the type via an only list (though since we don't allow operators to be named in limitation clauses, they couldn't write except + today, for instance).

Sidebar: This is a more motivated argument for allowing operators in an only or except list - in the case of use and import statements we're more likely to care on a type granularity rather than an operator granularity, but for forwarding that isn't a concern since we're already limiting it to a particular type.

bradcray commented 3 years ago

I definitely agree that if we forward operators (and I tend to think we should) that they need to be able to be listed in the only/except clauses for the forwarding statement (and that they should be itemizable in import/use for standalone cases as well).

For me, the concerns you raise don't seem particularly different than other forwarding scenarios for methods. I tend to think of forwarding as being a "savvy user" feature in general, so it doesn't bother me that including operators will give those users another class of things to think about (and I think that once operators are expressed more like methods, it's really not so different). As with forwarding methods, I think it feels much more attractive to give users the option to opt into forwarding all operators, or to reduce that set by opting in or out of others, than it does to always require them to explicitly define and forward each operator one by one. I expect that a conservative datatype developer will always explicitly list the things they want forwarding to support rather than relying on a catch-all, to avoid surprises.

I also have a vague recollection of @LouisJenkinsCS requesting operator forwarding for some user-defined int-like types that he was creating, but I'm not finding that reference in an issue on our repo with a quick search...

lydia-duncan commented 3 years ago

Does the following code seem like it has a reasonable expectation for forwarding and operator methods?

record Container {
  forwarding var x: OtherType;
}

record OtherType {
  var y: int;
}
proc type OtherType.+(lhs: OtherType, rhs: OtherType) {
  return new OtherType(lhs.y + rhs.y);
}

proc type OtherType.<(lhs: OtherType, rhs: OtherType) {
  return lhs.y < rhs.y;
}

proc main() {
  var a: Container = new Container(new OtherType(3));
  var b: Container = new Container(new OtherType(2));
  var c = a + b;
  writeln(c.type: string); // Expect this to be `Container`
  writeln(c); // Expect this to have had the contents of the two fields added, so `(x = (y = 5))`
  writeln(a < b); // Expect this to be `false` (because 3 > 2)
}
lydia-duncan commented 3 years ago

Or should we expect the type returned for + to be OtherType and for a proc type Container.+ definition to be required?

mppf commented 3 years ago

I'd think that, if we want to forward operators other than comparisons, then we need var c = a + b in your example above to result in c being a Container.

But, I don't think it's particularly clear exactly what approach the language would take to do that.

One interesting thought experiment is to think about what methods forwarding is creating on behalf of the programmer.

If I have

record Container {
  forwarding var x: OtherType;
}

and OtherType has a proc OtherType.foo(), then we can think of forwarding as adding (among other things):

proc Container.foo() {
  return OtherType.foo();
}

Now, what if we OtherType has a copy() function? We could imagine that we might expect it to forward like this:

proc Container.copy() {
  return new Container(x.copy());  // or maybe var ret: Container = x.copy() if we want to lean on init=
}

We could consider extending forwarding in this way - to automatically wrap any OtherType function result into a Container. This would technically be a breaking language change for forwarding (Because such a copy() function would now have a different return type from what it had previously). What is unclear to me is whether or not this would be reasonable in general or if it is something that one would need to opt into when needed. I can imagine functions that we want to return OtherType even when forwarded. I also don't know if there is any precedent for this kind of thing. FWIW, when introducing forwarding, Ruby was one of the languages with a precedent in the area that I considered.

lydia-duncan commented 3 years ago

Yeah, it's interesting! The situation also becomes more complicated if:

lydia-duncan commented 3 years ago

I'm realizing we have a similar problem with inheritance, but also that we could take a similar policy to what we do for initializers in that case. If my memory serves, that policy is "if it's simple, we generate a version that calls the parent one, and if that doesn't work we generate a warning telling the user that they don't get a compiler generated version", but I'm planning on double checking that.

And while we don't have an initializers forwarding idea (nor do I think we want to) we could probably extend similar guidance for forwarding.

bradcray commented 3 years ago

I'm coming late to this, but: When I see:

  var c = a + b;
  writeln(c.type: string); // Expect this to be `Container`

I'd expect c's type to be OtherType as the language is defined today. But I agree that forwarding would be more useful if there were some way to forward and then re-wrap the returned value to make it a Container again.

It seems as though some sort of initializer / copy initializer / forwarding initializer might be the right way to think of this because it's a "make a new Container out of an OtherType" pattern. I don't think I'd want it to be automatic/implicit, but it would be nice if it were trivial to opt into. One danger of making it implicit might be that if I happened to have an initializer / copy initializer that took a bools, I wouldn't want < results to automatically be turned into Container values. Maybe there's some sort of clause / suffix /argument we could put on forwarding to say when this upcast should be applied to the result? Brainstorming, it could be something as clunky as forwarding(upcast=true) or as subtle as forwarding= (where in either case, I'm imagining that Container has some sort of initializer that can turn an OtherType into a Container).

I think that an initializer-based approach would address both of Lydia's questions in this comment because the initializer would fill in the missing information and need to have the right signature to apply (as copy initializers need to today). The main question for me is whether this is just the presence of a copy initializer and/or 1-arg initializer, or whether it's a new thing in Michael's 4-layer cast/coercion/in/out hierarchy.

mppf commented 3 years ago

The main question for me is whether this is just the presence of a copy initializer and/or 1-arg initializer, or whether it's a new thing in Michael's 4-layer cast/coercion/in/out hierarchy.

I'm pretty sure it'd be sufficient for it to be one of the existing things, but I don't think we have to make that choice. I suppose that the difference is whether or not this pattern causes Container to be initializable/castable from OtherType. If we didn't want to associate them, we could make a type method on Container and have the forwarding mechanism call it.

For example, we could write

record Container {
  forwarding var x: OtherType;
  type proc forwardingWrapper(arg: OtherType): Container {
     return new Container(arg); // or however the type wishes to implement this
  }
}

Now when we call myContainer.copy(), the compiler would emit Container.forwardingWrapper(myContainer.x.copy()). The idea would be that if forwardingWrapper did not exist for a given type then the compiler wouldn't call it. E.g. for myContainer.isEqual() the compiler would look for Container.forwardingWrapper(bool) and not find it handle the forwarding as myContainer.x.isEqual().

Obviously we could come up with a better name / syntax. But what I like about this kind of approach is that the opting in to wrapping return values also describes which return values to wrap and how to do it.

A more syntax-y approach would be

record Container {
  var x: OtherType;
  forwarding x {
    proc return(arg: OtherType) { return new Container(arg);  }
  }
lydia-duncan commented 3 years ago

I think these approaches are interesting. But adding functions to support this case also strikes me as maybe defeating the purpose of forwarding? Like, is it better for the user to just have to write:

record Container {
  var x: OtherType;
  forwarding x except +;
  ...

  proc type +(lhs: Container, rhs:Container) {
     return new Container(lhs.x + rhs.x, ...);
  }
}

Granted, there's a difference between writing one function or block to enable a group of operators versus writing a wrapper for each of them and the former is definitely preferable. I guess my question is, at what point does adding additional mechanisms for control mean we're failing to provide the benefits we were hoping to provide?

bradcray commented 3 years ago

My argument would be: If a type supports a dozen operators, it'd be much nicer to be able to say something like:

forwarding= x only +, -, *, /, **, <, <=, >, >=, ==, !=;

than to write the three line pattern you gave a dozen times, particularly given how rote/redundant it is from one case to the next. I think the point of forwarding is to avoid re-implementing the core operator, which both your rewrite and mine do. But if the cost of that is the requirement to write a bunch of upcast boilerplate code, then the benefits are reduced.

I think this is easier to motivate by a less-abstract example. For example, if I had an age type:

record age {
  var myAge: int;
}

it'd be nice if any math I did on ages could return ages, but to implement that math by forwarding to int's operators. If I can do that with O(1) or O(op names) keystrokes rather than O(op wrapper routines), that seems like a major win (and a common case... the desire to upcast back to the original type).

lydia-duncan commented 3 years ago

Cycling back, here are what I think the major questions that should be resolved (and where I think we currently stand). My hope is that this comment is a good entrance point into the discussion.

  1. Should operator methods be forwarded?
    • It seems like we think they should
  2. For operator methods that return an instance of the type, should the forwarded version return an instance of the forwarded type or the containing type?
    • We think it's acceptable to return the forwarded type but would ideally return the containing type when possible. 2a. If the containing type, should this be automatic or something to opt in to? What would it look like to opt in?
    • It seems like we would like this to be opt in, to avoid accidentally turning operators that should always return a bool (like >, for instance) into operators that return the type if we happen to forward a bool field.
    • A couple of options have been thrown around for how to opt in. Seems like we would potentially want a way to specify which operators this should apply to, and/or which field or forwarded type it should apply to. Maybe the arguments to the operator will be required as well?
  3. Should any of this vary based on if we are forwarding a fully generic field (i.e. a field with no declared type)?
    • It seems like we think the answer is no
  4. Should we wait to allow this until forwarding only/except lists can name operators? (today forwarding field except +; is an error because + cannot be named in an except or only list, so users will not have the degree of control they would like)
    • I don't think we have a position on this yet.

Things to think about:

Additional context:

Precedent: Michael said that the forwarding feature was based off of Ruby. From a quick glance, the closest match is mixins, and that seems to assume that you'll want the value that is returned to maintain the same type. At least, all the examples are along those lines. Initializers: I mentioned how we generate calls to inherited initializers as a potential blueprint for this and inheriting operators. Unfortunately, I was remembering a feature that is not yet implemented (see #8232), so that's not as easy. There is still a general plan available, though it is associated with an older version of the initializer proposal so would probably need some updating.
bradcray commented 3 years ago

If the containing type, should this be automatic or something to opt in to? What would it look like to opt in?

I feel pretty strongly that this shouldn't be automatic. At least, it would feel weird to me for operators to do something automatic here, but for other methods that might return the type not to (where today, they don't).

bradcray commented 3 years ago

Should we wait to allow this until forwarding only/except lists can name operators? (today forwarding field except +; is an error because + cannot be named in an except or only list, so users will not have the degree of control they would like)

Either order here seems OK to me.

mppf commented 3 years ago

FWIW I'm happy with the "where we currently stand" leanings described above.

cassella commented 2 years ago

Similarly to handling a conversion from the return value of OtherType.op to the Container type, how are the arguments converted to the types the forwarded-to operator is expecting? E.g.,

var c1, c2: Container;

writeln(c1 + c2);

Where it'll look for Container.+, and not finding one, look through the forward to OtherType.+. But OtherType.+ presumably specifies its lhs and rhs as :OtherType. I'm assuming the compiler will insert code to adjust the callsite to match the forwarded method (notionally OtherType.+(c1.x, c2.x))?

If so, does it allow for writing c1 + o1 (where o1: OtherType)?

If there were another record type D forwarding to OtherType, would c1 + d1 work?

If there were a record ContainerContainer forwarding to a Container, could you write cc1 + c1 or cc1 + d1 or cc1 + o1?

(Assuming in all these cases that OtherType.+(lhs: OtherType, rhs: OtherType) is the only relevant + operator defined.)

I guess these all come down to, once the compiler's matched say the lhs of the operator to OtherType, how does it determine it can make the actual rhs into something it can pass to the OtherType operator's formal? Maybe getting kind of silly, but should the compiler be able to find its way through

record AnotherType {}
record OtherType { operator +(lhs: OtherType, rhs: AnotherType); }
record C { var x: OtherType; forwarding x; }
record D { var y: AnotherType; forwarding y; }

var c: C, d: D;
writeln(c + d);

Or is it only matching exactly on the formals being the forwarding type, and it can only, and will, pass the corresponding forwarded member?

Does any of this extend to non-operator methods, OtherType.foo(x: OtherType) allowing c1.foo(c2)?

lydia-duncan commented 2 years ago

Similarly to handling a conversion from the return value of OtherType.op to the Container type, how are the arguments converted to the types the forwarded-to operator is expecting? E.g.,

var c1, c2: Container;

writeln(c1 + c2);

Where it'll look for Container.+, and not finding one, look through the forward to OtherType.+. But OtherType.+ presumably specifies its lhs and rhs as :OtherType. I'm assuming the compiler will insert code to adjust the callsite to match the forwarded method (notionally OtherType.+(c1.x, c2.x))?

Yeah, it'll adjust the argument whose forwarding is used in the same way a normal method call would be adjusted.

If so, does it allow for writing c1 + o1 (where o1: OtherType)?

Yes. My branch currently allows that. In a little more detail, the approach it takes is to look at combinations involving forwarding for the arguments involved. E.g. say Foo forwarded to Bar, which forwarded to Baz, and Bar defined an operator with two arguments of type Bar. The following is the current combinations we will check:

Foo, Foo
 -> Bar, Foo
   -> Baz, Foo
   -> Baz, Bar
   -> Baz, Baz
 -> Bar, Bar - This is the match we're looking for
 -> Bar, Baz
Foo, Bar
Foo, Baz

If there were another record type D forwarding to OtherType, would c1 + d1 work?

Yup, because we're checking each argument's forwarding in combination with any forwarding that may exist for others.

If there were a record ContainerContainer forwarding to a Container, could you write cc1 + c1 or cc1 + d1 or cc1 + o1?

I think this is answered by my earlier point, let me know if you would like it called out more explicitly.

(Assuming in all these cases that OtherType.+(lhs: OtherType, rhs: OtherType) is the only relevant + operator defined.)

I guess these all come down to, once the compiler's matched say the lhs of the operator to OtherType, how does it determine it can make the actual rhs into something it can pass to the OtherType operator's formal? Maybe getting kind of silly, but should the compiler be able to find its way through

record AnotherType {}
record OtherType { operator +(lhs: OtherType, rhs: AnotherType); }
record C { var x: OtherType; forwarding x; }
record D { var y: AnotherType; forwarding y; }

var c: C, d: D;
writeln(c + d);

I believe yes.

Or is it only matching exactly on the formals being the forwarding type, and it can only, and will, pass the corresponding forwarded member?

We're not generating a new version of the forwarded operator with new argument types and calling that, I think mostly because we're not taking the tactic of adjusting the return type to match an expected return type for the wrapper type (at least, not yet and not without a way for the user to opt-in to doing so).

Does any of this extend to non-operator methods, OtherType.foo(x: OtherType) allowing c1.foo(c2)?

No, regular methods only forward on the receiver type

cassella commented 2 years ago

Thanks! That answers all my questions. I hadn't even realized that for regular methods, the this argument would of course have to have this same sort of treatment.