Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoiding this capture (use bond()) #19

Closed
michaelfig opened this issue Jan 10, 2019 · 11 comments
Closed

Avoiding this capture (use bond()) #19

michaelfig opened this issue Jan 10, 2019 · 11 comments

Comments

@michaelfig
Copy link
Member

I would like to propose that we remove the requirement for avoiding this capture by:

(1,some.method.here)(...args)

from Jessie. Discussion below, please.

@michaelfig
Copy link
Member Author

With a little thought, we could have something like wrapFn, which would be:

function wrapFn(fnOrValue) {
  if (fnOrValue instanceof Function) {
    return (...args) => fnOrValue(...args);
  }
  return fnOrValue;
}

Then, every time we assign something to an array or object (not Maps, Sets or other objects with functional interface), we would insert a call to wrapFn (or its inlined equivalent) unless we know statically that the value is not a function. This can be done by the compiler, just as you are proposing for harden in Jessie.

This would change method pointers to be more like Python: always carry the self around with the ponter, rather than Javascript where they can be detached.

Thanks,
Michael.

@erights
Copy link
Contributor

erights commented Jan 10, 2019

Excellent! As an statically enforced practice for what appears in Jessie, it is unquestionably an improvement.

As for a compiler transform, that Jessie would run as an embedded language within a SES host, rather than directly as a subset of SES. I was thinking of introducing this level separation for some purposes anyway, but ala carte. I still want to be able to run Jessie programs as SES programs within SES systems, with the constraint that a correct Jessie program remain correct in this scenario. I can do this with your static checking rule, but not if the transform is hidden within a Jessie->SES compiler.

@michaelfig
Copy link
Member Author

Okay, I understand better what you're hoping to achieve by running Jessie in SES.

Can I suggest the following:

const badRecord = {
 foo: bar.baz.bot[2],
 bar: other[2],
 baz: other.baz,
};
badRecord.bingo = foo.bot;
const badArray = [
  bar.baz
];
badArray[2] = other.ref[3];

const goodRecord = {
 foo: bond(bar.baz.bot, 2),
 bar: bond(other, 2),
 baz: bond(other, 'baz'),
};
goodRecord.bingo = bond(foo, 'bot');
const goodArray = [
  bond(bar, 'baz')
];
badArray[2] = bond(other.ref, 3);

All the bad* entries would cause a compiler error, something like:

Setting 'badRecord.foo' to reference 'bar.baz.bot[2]' which might be a method;
you must protect the reference with 'bond(bar.baz.bot, 2)' to prevent accidental 'this' capture of 'badRecord'

Here is a sample bond implementation:

let _bonded;

// Ensure that `maybeThis[index]` is not a method that could capture another context as `this`.
const bond = (maybeThis, index) => {
  const maybeMethod = maybeThis[index];
  if (maybeMethod instanceof Function && !_bonded.has(maybeMethod)) {
    const bondedMethod = (...args) => maybeThis[index](...args);
    _bonded.add(bondedMethod);
    return bondedMethod;
  }
  return maybeMethod;
};

_bonded = new WeakSet([bond]);

@erights
Copy link
Contributor

erights commented Jan 10, 2019

I like this.

A nit:

maybeMethod instanceof Function

is not a reliable test. I think you want

typeof maybeMethod === 'function'

@erights
Copy link
Contributor

erights commented Jan 10, 2019

Because of JavaScript's scoping rules, you can omit the let _bonded; at the top and just say

const _bonded = makeWeakSet([bond]);

at the bottom.

@michaelfig
Copy link
Member Author

Terrific, thank you! I'm adding this to the Jessica TODO.md:

foo.bar = other.obj.index;
(1,foo.bar)(baz)

in callers avoiding this-capture in favour of protecting callees:

foo.bar = bond(other.obj, 'index');
foo.bar(baz)

@michaelfig
Copy link
Member Author

michaelfig commented Jan 11, 2019

The last part of this is that we also need to bond function values (as opposed to references) that cannot be statically proven to be arrow functions. I suggest we use a single-argument bond in this case (index = undefined), and bind this to null.

Oh, and indexed-bond needs to be statically rejected for computed indices that aren't numbers. Unless they're a string literal. Otherwise, bond can bypass Jessie's restrictions on computed indices.

...And bond needs to reject attempts to call with a non-object (or null) first argument.

This is getting hairy. Is there something wrong with this API methodology, or is it just a complex subject?

You can follow my implementation in jessica for what I currently have.

@michaelfig michaelfig changed the title Avoiding this capture Avoiding this capture (use bond()) Jan 23, 2019
@dckc
Copy link
Contributor

dckc commented Jan 23, 2019

Then, every time we assign something to an array or object ..., we would insert a call to wrapFn ...

That seems like it would change the meaning of some programs...

Does this preserve the property that Jessie is absorbed by SES and JavaScript?

@michaelfig
Copy link
Member Author

@dckc, as @erights stated later, this would not be done by a compiler transform. Instead, it would be statically required by Jessie to have the developer insert calls to bond() just as calls to harden() are statically required.

bond() would be part of sesshim.js.

@dckc
Copy link
Contributor

dckc commented Jan 23, 2019

ok... so any programs where you might be able to observe a difference are not part of Jessie. Thanks.

@michaelfig
Copy link
Member Author

As proposed in #27, bond() is unnecessary if immunize() transitively hardens its functions' this values as well. Then, the only possible captured this is hardened objects.

I'm withdrawing this proposal in favour of immunize. We can discuss further there, and reopen this proposal if absolutely necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants