Automatically export runtime when necessary

[dcodeIO]

Looks at whether

• Either ESM or RAW bindings are to be generated
• Any external function or global desires an exported runtime

and if both is the case, automatically enables --exportRuntime.

• I've read the contributing guidelines
• I've added my name and email to the NOTICE file

[MaxGraey]

Any external function or global uses managed types

I think make export runtime only when we have lowering managed types. When managed types just lifted __pin and __new doesn't required

[dcodeIO]

That's true for most types, except for what we call internrefs. These require a __pin (via __retain) upon lifting.

[MaxGraey]

yeah. But will be great to don't export full runtime when this doesn't really necessary.

[dcodeIO]

A bit of a hen and egg problem, in that the exact lifts and lowers are only computed once bindings are generated. Need to check.

[MaxGraey]

I wonder if it is possible to just run the recompilation? If it requires serious refactoring, but it's probably not worth the hassle.

[dcodeIO]

Looked into this a little, and figured that checking during compile can become quite expensive. For example, if the lifted type is considered to be a plain object, there might be fields that lift as internref, but recursively checking every field seems wasteful. Would need some caching and whatnot. Recompiling seems wasteful as well, but at least doesn't require a bunch of extra code hmm.

[MaxGraey]

how about fast approximation checks? If module have exported function which has well known input types that required runtime like Array, TypedArray, String, Map, Set and also "Plain Object "(don't do recursively visiting just pessimistic assume it will contain managed object) we always export runtime.

[dcodeIO]

Approximate checks are about

  static liftRequiresRuntime(type: Type): bool {
    let clazz = type.classReference;
    if (!clazz) {
      assert(type.signatureReference);
      return true; // lifts using __pin
    }
    let program = clazz.program;
    // lifts via memory copy
    if (clazz.extends(program.arrayPrototype)) {
      return JSBuilder.liftRequiresRuntime(clazz.getArrayValueType());
    }
    return !(
      clazz.extends(program.arrayBufferInstance.prototype) ||
      clazz.extends(program.stringInstance.prototype) ||
      clazz.extends(program.staticArrayPrototype) ||
      clazz.extends(program.arrayBufferViewInstance.prototype)
    );
  }

  static lowerRequiresRuntime(type: Type): bool {
    let clazz = type.classReference;
    if (!clazz) {
      assert(type.signatureReference);
      return false; // lowers as Internref
    }
    // lowers using __new
    let program = clazz.program;
    return (
      clazz.extends(program.arrayBufferInstance.prototype) ||
      clazz.extends(program.stringInstance.prototype) ||
      clazz.extends(program.arrayPrototype) ||
      clazz.extends(program.staticArrayPrototype) ||
      clazz.extends(program.arrayBufferViewInstance.prototype)
    );
  }

[MaxGraey]

Looks good

[MaxGraey]

LGTM. Could also add couple simple tests which may prevent some regressions in future? It could be just 2 tests:

1. Some non-references types + some ref types which shouldn't trigger exportRuntime
2. references types which should definitely trigger exportRuntime

[dcodeIO]

Tests here are rather clumsy I figured. The test for "don't export runtime" is OKish while not adding much value, while the inverse tests require one file per type with basically a single line of code that then produces a full runtime-enabled fixture that can only be reasonably checked visually and is likely to change frequently due to unrelated changes. Seems to produce more inconvenience than it takes away.

[MaxGraey]

Ok, could you add that test which doesn't trigger exportRuntime. And just remove explicit exportRuntime flag from existing binding tests?

[MaxGraey]

Also add warning about --exportRuntime deprecation

[dcodeIO]

Hmm, won't we still need it for other cases, i.e. where bindings are not desired?

[MaxGraey]

Hmm, won't we still need it for other cases, i.e. where bindings are not desired?

I guess in this case we could add tip in docs which suggest explicitly export __pin, __unpin and __new due to this is rare scenarios for now. But I'm pretty sure that's what custom bindings do. By removing exportRuntime we reduce the number of flags for compiler

[dcodeIO]

Seems like a good goal to make this all explicit, yet gets into breaking change territory I think. Will need to let this sink a little, and suggest to leave further changes to a follow-up. Perhaps there's even more we can remove on this basis.

[MaxGraey]

Seems like a good goal to make this all explicit, yet gets into breaking change territory I think

No, if it will only deprecation warning

[dcodeIO]

Right, a deprecation warning for a breaking change we don't yet know for sure whether (or when) to make. Seems too early to me.

[MaxGraey]

Alright

[dcodeIO]

Are you OK with the tests? Is a little clumsy I admit :)

[MaxGraey]

Well, it's better than nothing =)

[MaxGraey]

Could you sync the pr with main?