xs throws GeneratorFunction.__proto__ was not Function.prototype.constructor

[dckc]

Does this mean xs is non-conforming? Or just that it's not a platform that this code knows about?

https://github.com/Agoric/harden/blob/0a5185ee224b98e75623a2f88143d118e0849eaa/src/anonIntrinsics.js#L230

for reference, my main.js:

import harden from '@agoric/harden';

export default function main() {
  trace("hi there\n");
  const h = harden({});
  trace("harded:", String(h), "\n");
}

and my manifest.json:

{
  "include": [
    "$(MODDABLE)/examples/manifest_base.json",
  ],
  "modules": {
    "*": [
      "./main",
    ],
    "@agoric/make-hardener": "./node_modules/@agoric/make-hardener/index",
    "@agoric/harden": "./node_modules/@agoric/harden/dist/harden.esm",
  },
}

[dtribble]

The test seems wrong:

> const x = function*(){}
undefined
> x
ƒ* (){}
> x.__proto__
GeneratorFunction {prototype: Generator, constructor: ƒ, Symbol(Symbol.toStringTag): "GeneratorFunction"}
> x.__proto__ === Function.prototype.constructor 
false

[dckc]

After I disable that test and a couple others, I hit

      if (whiteTable.has(value)) {
        throw new Error('primordial reachable through multiple paths');

I don't know if that's

• a consequence of disabling these checks
• a bug in xs
• a bug in harden
• something else I can't think of

the others:

1. getProto(AsyncGeneratorFunction) !== Function.prototype.constructor
2. getProto(AsyncFunction) !== Function.prototype.constructor

[dtribble]

There should be info on the multiple paths it is reachable through.

[erights]

@dckc

Does this mean xs is non-conforming? Or just that it's not a platform that this code knows about?

This likely means that XS is non-conforming.

@dtribble

The test seems wrong:

You're misreading the test. GeneratorFunction is not a generator function. It is the GeneratorFunction constructor, analogous to Function being the Function constructor.

In a conforming JS, Function.prototype.constructor === Function. The GeneratorFunction constructor is supposed to inherit from the Function constructor, emulating class-side inheritance.

To understand this bug better, what is the value of GeneratorFunction.__proto__ when this line is executed?

The multiple paths error is wrt how things are named in the whitelist. It is indeed an error if something is reachable through multiple paths through the whitelist's naming.

[dckc]

@erights asks:

To understand this bug better, what is the value of GeneratorFunction.__proto__ when this line is executed?

Um... it prints as:

@@value: function  (){[native code]}

when I add...

trace(`@@value: ${GeneratorFunction.__proto__}\n`);

I'm not sure how to write down "the value" of such a thing.

[dckc]

Poring over the source code I found pretty clear evidence that the two prototypes are different by design:

enum {
...
	mxFunctionPrototypeStackIndex,
...
	mxAsyncFunctionPrototypeStackIndex,
	mxGeneratorPrototypeStackIndex,

https://github.com/Moddable-OpenSource/moddable/blob/c92dff582f4f439bd25ac8eb6c6109d045a307f2/xs/sources/xsAll.h#L2217-L2218

@mkellner @phoddie care to comment on this code vs. the observation by @erights ?

In a conforming JS, Function.prototype.constructor === Function. The GeneratorFunction constructor is supposed to inherit from the Function constructor, emulating class-side inheritance.

p.s.

I found those two lines via:

#define mxGeneratorPrototype the->stackPrototypes[-1 - mxGeneratorPrototypeStackIndex]

https://github.com/Moddable-OpenSource/moddable/blob/c92dff582f4f439bd25ac8eb6c6109d045a307f2/xs/sources/xsAll.h#L2394

which I found from:

void fxBuildGenerator(txMachine* the)
{
...
	mxGeneratorPrototype = *the->stack;

https://github.com/Moddable-OpenSource/moddable/blob/public/xs/sources/xsGenerator.c#L58

[dckc]

@dtribble

There should be info on the multiple paths it is reachable through.

Adding some debug tracing to harden.esm.js suggests a collision between GeneratorFunction and ThrowTypeError. I'm not sure what to make of that.

tweaked harden.esm.js and the full trace are in
https://gist.github.com/dckc/367ec21c261fc80ab77a8846568c4f9e

relevant excerpt of the trace:

@adding globals: Infinity,NaN,undefined,...,Realm,SES,Nat,def
@@recur: register globals>Infinity
@@recur: register globals>NaN
...
@@recur: register globals>Object>defineProperties
...
@@recur: register globals>unescape
@adding intrinsics: ThrowTypeError,...,GeneratorFunction,AsyncGeneratorFunction,AsyncFunction,TypedArray
@@recur: register intrinsics>ThrowTypeError
@@recur: register intrinsics>IteratorPrototype
...
@@recur: register intrinsics>SetIteratorPrototype
@@recur: register intrinsics>GeneratorFunction
COLLISION: intrinsics,ThrowTypeError
/home/connolly/projects/agoric/SwingSet/node_modules/@agoric/harden/dist/harden.esm.js (1337) # Break: throw!
}}}

[phoddie]

Regarding this....

In a conforming JS, Function.prototype.constructor === Function.

...yes, and that is the case in XS. Test262 has a test for this which XS passes.

That said, there are several modes for executing JavaScript. The Moddable SDK configures XS to run with Compartments. In that case, code running as "vetted customization code" executes outside Compartments, and so the test above evaluates to true. Once that phase is complete, code executes from the start compartment. As Mark wrote in the standalone SES specification:

Each compartment scope has its own Function, which does evaluate. All compartment scopes share the same Function.prototype and therefore the same Function.prototype.constructor which is a function that only throws. Thus, in all compartment scopes,

Function !== Function.prototype.constructor

Regarding this...

The GeneratorFunction constructor is supposed to inherit from the Function constructor, emulating class-side inheritance.

...Mark's words are very precise, as usual. Mark wrote "is supposed to" instead of "is required to." ECMA-262 does not require this inheritance. There is a non-normative diagram in Section 25.2 of ECMA-262 which states "%GeneratorFunction% is essentially a subclass of Function." The spec text does not require it to be, and test262 does not contain a test for that. It appears to be an implementation choice left to the engine. It appears that different engines have made different choices here.

Regarding this...

Poring over the source code I found pretty clear evidence that the two prototypes are different by design

...I'm unsure what this is intended to demonstrate. There are different prototypes for Function, AsyncFunction, and GeneratorFunction in ECMA-262. The implementation of XS reflects that.

[dckc]

Poring over the source code I found pretty clear evidence that the two prototypes are different by design

...I'm unsure what this is intended to demonstrate.

It seems to demonstrate that I'm lost.

Starting over... regarding the main.js above (and below):

1. Should it work (i.e. print two trace messages) outside a compartment?
   • If so, any suggestions on how to get it to do so?
     • when running on xs, disable the 4 checks that fail?
   • If not, what should it do?
     • Does SES or Agoric/SwingSet need harden to work outside a compartment? (I seem to recall @warner wrote something saying it would be nice, at least for testing.)
2. Should it work inside a compartment?
   a. Any suggestions on how to get it to do so? (I haven't actually tried this yet...)

import harden from '@agoric/harden';

export default function main() {
  trace("hi there\n");
  const h = harden({});
  trace("harded:", String(h), "\n");
}

[dckc]

wait... is main.js actually running in a Compartment after all? Maybe I should arrange for makeHardener to run as a preload thingy?

[phoddie]

Here's a trivial example of main.js intended to be used with preload enabled for its module.

const testPreload = (function(){}).constructor === Function;

export default function() {
	trace(`(testPreload: ${testPreload}\n`);

	const test = (function(){}).constructor === Function
	trace(`(function(){}).constructor === Function: ${test}\n`);
}

The testPreload variable is evaluated at preload time and is consequently true. The same expression is evaluated in the start compartment when the default function is executed and consequently evaluates to false.

[dckc]

My first attempt to preload the harden stuff fails with a diagnostic that I find opaque; any clues what the actual failure mode is here? (I guess I should move this to a moddable support forum...)

$ mcconfig -d -m -p lin
# xsl modules
# mcrez resources
Total resource size: 0 bytes
### SyntaxError: eval: invalid script!

p.s. I see that this preload / compartment stuff is documented pretty well in https://github.com/Moddable-OpenSource/moddable/blob/public/examples/js/compartments/readme.md so thanks for holding my hand...

[dckc]

FWIW, disabling these checks seems to work (along with one other issue: https://github.com/Agoric/make-hardener/issues/47).

WARNING: meant to preload
WARNING: skipping test: GeneratorFunction.__proto__ was not Function.prototype.constructor
WARNING: skipping test: GeneratorFunction.__proto__ was not Function.prototype.constructor
WARNING: skipping test: AsyncFunction.__proto__ was not Function.prototype.constructor
COLLISION: intrinsics>GeneratorFunction = intrinsics>ThrowTypeError
WARNING: primordial reachable through multiple paths
COLLISION: intrinsics>AsyncGeneratorFunction = intrinsics>GeneratorFunction
WARNING: primordial reachable through multiple paths
COLLISION: intrinsics>AsyncFunction = intrinsics>AsyncGeneratorFunction
WARNING: primordial reachable through multiple paths
hi from main, take 21241
harded:[object Object]
done

[erights]

...Mark's words are very precise, as usual. Mark wrote "is supposed to" instead of "is required to." ECMA-262 does not require this inheritance.

Hi @phoddie thanks for the vote of confidence! Unfortunately, you overestimate me ;). I actually thought it was required and I spoke imprecisely. But I will try harder to live up to these high expectations!

There is a non-normative diagram in Section 25.2 of ECMA-262 which states "%GeneratorFunction% is essentially a subclass of Function." The spec text does not require it to be, and test262 does not contain a test for that. It appears to be an implementation choice left to the engine. It appears that different engines have made different choices here.

I find that unfortunate. In general, we're trying to remove as much non-determinism from the spec as possible. Do you see a reason to allow this to differ between implementations?

That still wouldn't address the major topic of this issue, about which more in a moment.

[erights]

At https://github.com/Agoric/harden/issues/47#issuecomment-521553588 I wrote:

In a conforming JS, Function.prototype.constructor === Function.

At https://github.com/Agoric/harden/issues/47#issuecomment-522341574 @phoddie responded:

yes, and that is the case in XS. [...] As Mark wrote in the standalone SES specification: [...]

Yes. I was reading the code against the assumption that it was executing in a non-SES environment. In a SES environment, the test and the error message as written should still be true, referring to Function.prototype.constructor rather than Function. SES should specify that all the other function constructors must directly inherit from Function.prototype.constructor. @phoddie do you see a reason not to require this?