Skip to content

Weird behavior with constructor parameter properties #1225

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

Closed
surma opened this issue Apr 14, 2020 · 5 comments · Fixed by #1255
Closed

Weird behavior with constructor parameter properties #1225

surma opened this issue Apr 14, 2020 · 5 comments · Fixed by #1255
Labels

Comments

@surma
Copy link
Contributor

surma commented Apr 14, 2020

This was a weird one to track down. Here’s a reduced reproduction.

class X {
  public normal: i32;
  public viaThis: i32;
  constructor(private x: i32) {
    this.viaThis = this.x;
    this.normal = x;
  }
}

const x = new X(4);
export function normal(): u32 {
  return x.normal;
}
export function viaThis(): u32 {
  return x.viaThis;
}
console.assert(instance.exports.normal() === 4);
// This will throw. With asc 0.9.4 it will return `0`
console.assert(instance.exports.viaThis() === 4); 

Fun fact: If you swap the assignments in the constructor, both return 4 🤷

@MaxGraey
Copy link
Member

Can confirm

@MaxGraey MaxGraey added the bug label Apr 23, 2020
@dcodeIO
Copy link
Member

dcodeIO commented Apr 23, 2020

Issue seems to be that when we compile

this.viaThis = this.x

we first compile this.x, which allocates this, and only then wrap it with something that needed the allocation to be performed earlier.

@dcodeIO
Copy link
Member

dcodeIO commented May 6, 2020

This is taking a little longer since I figured that there are even more problems than that, like this allocations leaking in mixed return from ctor scenarios. So far it looks like the current lazy allocation mechanism causes more problems than it solves, so investigating alternatives.

@dcodeIO
Copy link
Member

dcodeIO commented May 6, 2020

Quick update: So I thought I had a solution for this, but turned out the codegen is so hacky that it doesn't play well with the ARC finalization passes we have in Binaryen right now. Quite the conflict party between ARC, inlining, autoreleases and whatnot due to automatic allocation semantics of ctors in conjunction with the need to also allow returning explicitly within.

@dcodeIO
Copy link
Member

dcodeIO commented May 6, 2020

Binaryen PR that should help: WebAssembly/binaryen#2833

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

Successfully merging a pull request may close this issue.

3 participants