v12 alpha: incorrect compiler output

[cknitt]

When compiling rescript-schema with v12 alpha.3, I noticed invalid output. Simplified, the problem is this:

In v11, the following ReScript code:

type error

let reason = (error: error, ~nestedLevel as _ = ?): string => String.make(error)
let reason: error => string = Obj.magic(reason)

let message = (error: error) => `Failed reason: ${error->reason}`

compiles to

function reason(error, param) {
  return String(error);
}

function message(error) {
  return "Failed reason: " + reason(error);
}

whereas in v12 alpha, it compiles to the invalid code

function reason(error, param) {
  return String(error);
}

function message(error) {
  return "Failed reason: " + param => reason(error, param);
}

/cc @cristianoc @cometkim

[cometkim]

Hmm, that's confusing. Isn't this a change in type rather than a change in output? Let me find out.

[cometkim]

v12 alpha.1 output

function reason(error, param) {
  return String(error);
}

function message(error) {
  return "Failed reason: " + (function (param) {
    return reason(error, param);
  });
}

[cknitt]

So there seem to be two different issues:

1. Since alpha.2: The output is invalid JS (parenthesis missing).
2. Since alpha.1: The output is incorrect, we want to append reason(errror), not a function.

The latter may be related to some changes around removing curried mode?

[cometkim]

Since alpha.2: The output is invalid JS (parenthesis missing).

well, I think it shouldn't be able to produce without %raw, and %raw block will be wrapped correctly

[cometkim]

The latter may be related to some changes around removing curried mode?

This one: https://github.com/rescript-lang/rescript-compiler/commit/f603cf1779e08544e26e1c53925f3130294cc72b

[cknitt]

Played a bit more with this:

let f1: string => string = Obj.magic("not actually a function, but never mind")
let f2: string => string = Obj.magic((a, b) => a ++ b)
let f3: string => string = Obj.magic((a, b, c) => a ++ b ++ c)

let x1 = f1("test")
let x2 = f2("test")
let x3 = f3("test")

outputs

let f1 = "not actually a function, but never mind";

function f2(a, b) {
  return a + b;
}

function f3(a, b, c) {
  return a + b + c;
}

let x1 = f1("test");

function x2(param) {
  return f2("test", param);
}

function x3(param, param$1) {
  return f3("test", param, param$1);
}

in v12 alpha.

So somehow even though the type of each function is string => string after "casting" with Obj.magic, the original arity of the function is preserved and affects the generated code.

[fhammerschmidt]

I tried a slightly modified example in the playground with v10:

type error

let reason = (. error: error, ~nestedLevel as _): string => error->Obj.magic
let reason: error => string = Obj.magic(reason)

let message = (error: error) => `Failed reason: ${error->reason}

which gives me v10:

function message(error) {
  return "Failed reason: " + (function (param) {
            return reason(error, param);
          }) + "";
}

v11:

function message(error) {
  return "Failed reason: " + reason(error);
}

v12:

function message(error) {
  return "Failed reason: " + param => reason(error, param);
}

[cristianoc]

First, simplified example:

let first = (x, y) => x
let hack: int => int = Obj.magic(first)
let test = x => hack(x)

It's pretty questionable whether this should work, but one can see why the behaviour is like it is and what changed in v12. For sure, moving hack to another file would make it impossible to even know that there exists first underneath and the issue would go away.

[cristianoc]

The generated code looks fine on master. But not on playground alpha.3 https://rescript-lang.org/try?version=v12.0.0-alpha.3&code=FAGwpgLgBAZglgJwM7QLxQBQA8A0UCeAlFKgHxRaiRQAWAhgMYDWAXFHAHZrmdpQDyAIwBWAOgC2dAOZwGGeMgiEq0CGBQkKJcvWbZCQA

% ./bsc tst.res
// Generated by ReScript, PLEASE EDIT WITH CARE
'use strict';

let Obj = require("rescript/lib/js/obj.js");

function first(x, _y) {
  return x;
}

let hack = Obj.magic(first);

function test(x) {
  return hack(x);
}

exports.first = first;
exports.hack = hack;
exports.test = test;
/* hack Not a pure module */

@cknitt was this fixed?

[cknitt]

@cristianoc No, it was not fixed. I can reproduce it with your example if I add your example to tests/tests/src and run node ./scripts/test.js -node.

I get

function test(x) {
  return param => first(x, param);
}

then.

[cristianoc]

We have a couple of small issues. One is the bsc commands used are a bit opaque, and verbose mode should show them in detail. The other is rescript does not use default options while it should.

Here's the repro:

./bsc -bs-cross-module-opt tst.res

If rescript always uses that option, then it should be the default.

[cristianoc]

So what's going on is that it's inlining the function, so it kind of does what you tell it to do.

[cristianoc]

Here's the "fix":

let opaque_cast =  x => Obj.magic(x)

let first = (x, _y) => x
let hack: int => int = opaque_cast(first)
let test = x => hack(x)

[cknitt]

Hmm, -bs-cross-module-opt is not the default though. In the compiler repo, it is set explicitly in bsc-flags when building the runtime + tests.

But as far as I can see, it is not set in rescript-schema.

[cristianoc]

Hmm, -bs-cross-module-opt is not the default though. In the compiler repo, it is set explicitly in bsc-flags when building the runtime + tests.

But as far as I can see, it is not set in rescript-schema.

Then we should be consistent, and use defaults in tests, unless we are explicitly testing the non-default options.

[cknitt]

Then we should be consistent, and use defaults in tests, unless we are explicitly testing the non-default options.

Agreed! And this unearthes other interesting stuff: https://github.com/rescript-lang/rescript-compiler/pull/7071

[zth]

Is there a reason cross module opt isn't the default?

[cknitt]

Is there a reason cross module opt isn't the default?

It is labeled as experimental

    "-bs-cross-module-opt", set Js_config.cross_module_inline, 
    "*internal* Enable cross module inlining(experimental), default(false)";

[zth]

Yes but does it need to be?