Skip to content
This repository was archived by the owner on Feb 22, 2018. It is now read-only.

Map named arguments to es6 destructuring #180

Closed
vsmenon opened this issue May 12, 2015 · 10 comments
Closed

Map named arguments to es6 destructuring #180

vsmenon opened this issue May 12, 2015 · 10 comments

Comments

@vsmenon
Copy link
Contributor

vsmenon commented May 12, 2015

@jmesserly has suggested mapping Dart named parameters to ES6's new destructuring pattern. This is not yet implemented in Chrome / V8 AFAIK.

E.g., map this Dart:

void foo(p1, p2, {p3: 0, p4}) {
  ...
}

to this ES 6:

function foo(p1, p2, { p3=0, p4=null } = {}) {
  ...
} 
@vsmenon
Copy link
Contributor Author

vsmenon commented May 12, 2015

A nice writeup on ES 6 destructuring:

http://www.2ality.com/2015/01/es6-destructuring.html

@vsmenon
Copy link
Contributor Author

vsmenon commented May 12, 2015

@lrhn is exploring the idea of allowing both optional-positional and optional-named parameters in the same Dart function. Currently that is not allowed by the language.

It should work with our current scheme ... with perhaps a more complicated check. Can it work with destructuring? At first glance, it seems problematic as the ES6 function needs to know which param - positionally - to destructure.

We would need a calling convention that:

  • is consistent across across function subtyping / overriding (e.g., below, C::foo must be compatible with B::foo which must be compatible with A::foo)
  • has the named param in a fixed slot
  • is readable
class A {
  void foo(x, y, z);
}

class B extends A {
  void foo(x, y, z, {a});
}

class C extends B {
  void foo(x, y, [z], {a});
}

Some options:

  • Make the named param the first slot. This would need to be done defensively (e.g., A::foo, even though it doesn't have a named param).
  • Make the named param immediately follow the required params. This may require disallowing certain function subtype allowing optional-positional in place of required (e.g., C::foo's z).

Other ideas?

@jmesserly
Copy link
Contributor

I wonder if this could be pay-per-use, in other words, could we use destructuring in most places and only skip it when you're in an overridden/implemented method with an "incompatible" signature? That seems like an acceptable trade off.

e.g. in the example above, C.foo would have to understand named args can be passed at any point. We could just generate:

class C extends B {
  foo(x, y, ...args) {
    let {z = null, a = null} = dart.optional(args, 'z' /* positional arg list */);
  }
}

lots of room to play with the pattern, but basically, we can use destructuring inside the method too, combined with runtime helpers we get a lot of flexibility (at some perf/readability cost).

My hunch is all existing call sites/types are fine, and only when this new feature is used, in a derived class, we might suddenly need to do more work to figure out what happened. But we can detect that reliably at compile time (or runtime, via dynamic helpers), even for separate compilation, so seems okay?

@jmesserly
Copy link
Contributor

btw, the = null above was just to illustrate where default values can go.

@vsmenon
Copy link
Contributor Author

vsmenon commented May 20, 2015

We (@jmesserly @leafpetersen @jacob314) discussed this a little further. The invariant that is useful for DDC to know - for statically resolvable calls - is which slot the named parameter object is in. We could make it the first, but that is a big readability hit.

OTOH, we could make it the Nth - if we restricting subtyping rules on functions with named parameters to disallow any new optional positional parameters in a subtype.

E.g., this is legal:

class A {
  void foo(x, y);
}
class B {
  void foo(x, [y], {n});
}

but this is not - it's an invalid override of the above:

class C extends B {
  void foo(x, [y, z], {n});
}

@lrhn @munificent - is this a reasonable restriction?

@munificent
Copy link
Contributor

I could be wrong, but my hunch is that would end up being an unfortunate restriction. If we can't come up with another solution, I think it's probably worth hacking up some code we can run over a corpus to see if anything were to break under that limitation.

I like @jmesserly's idea of doing the destructuring in two stages if needed.

@vsmenon
Copy link
Contributor Author

vsmenon commented May 21, 2015

@munificent it's not a backwards breaking restriction, as it only affects sub-typing on function types that are disallowed right now - ones with both optional positional and named parameters. Not sure how we could test existing code.

@jmesserly perhaps the two-stage destructuring will work for function subtyping too? Can we always pass in the named param obj at the end and do something like:

foo(x, y, ...args) {
  let {z = null, a = null} = dart.optional(args[args.length-1]);
}

?

@jmesserly
Copy link
Contributor

I think it's probably worth hacking up some code we can run over a corpus to see if anything were to break under that limitation.

it's not a backwards breaking restriction, as it only affects sub-typing on function types that are disallowed right now - ones with both optional positional and named parameters. Not sure how we could test existing code.

Right, nothing would break under @vsmenon's restriction, because named and optional args can't be used together currently in Dart, at all. His proposal would strictly increase the number of acceptable combinations, just not go as far as the full DEP. But yeah, it would be interesting to know how often number of optional args is changed in subtypes or conversions between function types. In those cases, it would not be possible to add a new named argument under this restriction, so you couldn't take advantage of the new feature.

I like @jmesserly's idea of doing the destructuring in two stages if needed.

perhaps the two-stage destructuring will work for function subtyping too?

Not sure I fully grok that, but it sounds interesting :). Note we still need positional arg name:

let {z = null, a = null} = dart.optional(args, 'z');

that's so it can return an object: { z: <value of z>, a: <value of a> } which we then destructure. We could destructure an array, but then we'd need to know expected order of named args instead:

// z is optional positional, a and b are named
let [z = 1, a = 2, b = 3] = dart.optional(args, 'a', 'b');

I still have the feeling we only need the two-stage destructure when you actually mix them in the same method or function. Needs a proof though :). Could be some case I'm not thinking of.

Oh, I'm assuming we can figure out if the last arg is the named parameters. We'd do this by looking for a "plain" JS object literal, basically obj.__proto__ == Object.prototype. That's totally reliable for Dart->Dart calls, but it does mean in JS->Dart interop scenarios, we might interpret a structurally typed JS object as named args, when it was supposed to be an optional positional. Conversely, we'd reject an *Options if it was actually an ES class. Granted, a JS API that wanted to accept optional positional and named arguments would probably have the same issue. But folks don't really design JS APIs like that. So that's one objection I have, these would never really be exportable to JS. Maybe we need a way in DDC to mark "exported" APIs so we can forbid this pattern on those functions.

Anyway, I don't personally feel like the sky is falling, but, it'd be nice to get a little further along in dev_compiler before tackling this. There's a lot of value to folks DDC could deliver before we get there. But philosophically, not sure any of this should hold up a language DEP. Worst case, it's just something you can't use if you want to use DDC, but you could use if you're targeting other Dart platforms.

@vsmenon vsmenon assigned vsmenon and ochafik and unassigned vsmenon Nov 30, 2015
ochafik pushed a commit that referenced this issue Nov 30, 2015
Before:
let onMatch = opts && 'onMatch' in opts ? opts.onMatch : null;
let onNonMatch = opts && 'onNonMatch' in opts ? opts.onNonMatch :
null;
After:
let {onMatch = null, onNonMatch = null} = opts || {};
@ochafik
Copy link
Contributor

ochafik commented Nov 30, 2015

With support for ES6 destructuring in current Chrome Canary, we can use the following syntax for named parameters without changing our ABI:

Dart:

f(a, {b, c: d}) {}

f(1, b: 2, c: 3);

ES6:

function f(a, {b = null, c = d} = {}) {}

f(1, {b: 2, c: 3});

(we could also give a null default value to a to avoid getting spurious undefined values)

As a reference point, here's our current output:

function f(a, opts) {
  let b = opts && 'b' in opts ? opts.b : null;
  let c = opts && 'b' in opts ? opts.b : d;
}

ochafik pushed a commit that referenced this issue Dec 2, 2015
# The first commit's message is:
Use destructuring assignments for named parameters (#180)

Before (for `f(a, {b, c: c_default})`):
  function f(a, opts) {
    let b = opts && 'b' in opts ? opts.b : null;
    let c = opts && 'c' in opts ? opts.c : c_default;
    ...

After:
  function f(a, {b = null, c = c_default} = {}) {
    ...

Note:
- Still reverting to old code when any parameter clashes with reserved JS names (see discussion in #392)
- When a parameter clashes with a Object.prototype property, using a clean default opts value (Object.create(null)).
- Passing opts through in aliased constructors, both for speed/concision and correctness (since default param value semantic is weird there)

# This is the 2nd commit message:

Rename Destructuring to BindingPattern

# This is the 3rd commit message:

Revert parseLeftHandSide + parseVariableBinding

# This is the 4th commit message:

Drop out of destructuring when names clash with js reserved names
@ochafik
Copy link
Contributor

ochafik commented Dec 2, 2015

(Mostly?) fixed by c9d909c :-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

4 participants