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

Multiline control externs broken by JS semicolon insertion #569

Open
marzipankaiser opened this issue Sep 3, 2024 · 4 comments · May be fixed by #589
Open

Multiline control externs broken by JS semicolon insertion #569

marzipankaiser opened this issue Sep 3, 2024 · 4 comments · May be fixed by #589
Labels
area:js bug Something isn't working

Comments

@marzipankaiser
Copy link
Contributor

When the user defines a JS extern with control effects like so:

extern control def foo(): Int = 
  js """
     $effekt.pure(2)
  """

Then the call fails at runtime with an error like:

    foo_2839().then((v_r_2852_3977) =>
              ^

TypeError: Cannot read properties of undefined (reading 'then')
    at Object.head (/Users/gaisseml/dev/effekt/experiments/out/js_multiline_externs.js:828:15)
    at apply (/Users/gaisseml/dev/effekt/experiments/out/js_multiline_externs.js:459:25)
    at Object.apply (/Users/gaisseml/dev/effekt/experiments/out/js_multiline_externs.js:549:34)
    at trampoline (/Users/gaisseml/dev/effekt/experiments/out/js_multiline_externs.js:435:19)
    at Object.run (/Users/gaisseml/dev/effekt/experiments/out/js_multiline_externs.js:542:18)
    at Object.main (/Users/gaisseml/dev/effekt/experiments/out/js_multiline_externs.js:836:27)
    at Object.<anonymous> (/Users/gaisseml/dev/effekt/experiments/out/js_multiline_externs:2:79)
    at Module._compile (node:internal/modules/cjs/loader:1480:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1564:10)
    at Module.load (node:internal/modules/cjs/loader:1287:32)

Node.js v22.1.0
[error] Process exited with non-zero exit code 1.

Expected behaviour

This should return 2.

Why does this happen

The above definition gets translated to the following JS:

function foo_2839() {
  return 
       $effekt.pure(2)
    ;
}

Semicolon insertion will insert a ; after the return, so this is equivalent to something like:

function foo_2839() {
  return undefined; // early return
  $effekt.pure(2); // dead code
  ;
}

Full example code

extern control def foo(): Int = 
  js """
     $effekt.pure(2)
  """

extern control def bar(): Int = 
  js """$effekt.pure(1)"""


def main() = {
  println(bar())
  println(foo())
}
@marzipankaiser marzipankaiser added bug Something isn't working area:js labels Sep 3, 2024
@marzipankaiser
Copy link
Contributor Author

Possible solution ideas

  • Drop leading (and potentially trailing, for aesthetics) whitespace when generating the code
  • Insert parentheses (does this help?? does this still allow everything we want to allow?)

@marzipankaiser
Copy link
Contributor Author

  • Discovered by @phischu earlier today.
  • @jiribenes do you think this will be a problem for your Effekt practical?

@b-studios
Copy link
Collaborator

b-studios commented Sep 3, 2024

Would it solve the problem to turn the space after return into a nonbreaking one?

case Return(expr) => "return" <+> toDoc(expr) <> ";"

Edit: Ahh, I see, it's the whitespace of the extern...

@b-studios
Copy link
Collaborator

b-studios commented Sep 3, 2024

Here is a proposal:

in the JS pretty printer strip whitespace and check that they only have one line or error otherwise.

This way we can see, whether we ever use them and react then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:js bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants