Skip to content

Rework constructor handling #446

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

Merged
merged 9 commits into from
Feb 2, 2019
Merged

Rework constructor handling #446

merged 9 commits into from
Feb 2, 2019

Conversation

dcodeIO
Copy link
Member

@dcodeIO dcodeIO commented Jan 31, 2019

As a follow-up to #445 this PR aims at reworking constructor handling. Currently, whenever a new is encountered, it calls the constructor if it is present and if it isn't, inlines a bare allocation with field initializers at that place. While we could get away with this, generating actual constructors for classes that don't explicitly define one would simplify fixing some remaining issues, while also reducing code size (inlined allocation and initializers quickly accumulate), instead of just working around them.

@dcodeIO
Copy link
Member Author

dcodeIO commented Feb 1, 2019

Alright, this is passing the tests now. Changes:

  • If a class doesn't have a declared constructor, one is generated that contains both a conditional allocation (if this=0 was provided) to allocate the necessary memorym plus field initializers in any case. This allows providing this!=0 if the memory has already been allocated earlier (just as before), i.e. in a derived constructor.
  • When super() is called, it calls the now guaranteed to exist base constructor, but first allocates the necessary memory for this which might be larger due to additional own fields, and then initializes own fields (TODO: this can be optimized if there are no own fields)

Other notes:

  • In order to dynamically generate functions that have an IR representation, a NATIVE_CODE element in builtins has been introduced to represent them. A bit hacky, but doing all that in pure code would be huge.
  • Decided to keep tracking of allocations in flows, for example for when a constructor can both return something manually instantiated or use this normally. Doing that has a cost in code size when this is first accessed in separate branches, though, because there's one conditional allocation and field initializers in each branch then (whenever this is fist accessed). This can probably be optimized somehow if all branches are known to allocate, but since code is compiled sequentially at this point, there's not yet a way to know that beforehand.
  • This doesn't bother with inlining yet due to the existence of Proposing user-defined inlining WebAssembly/binaryen#1889.

@@ -1023,150 +1022,175 @@ export class Compiler extends DiagnosticEmitter {
return typeRef;
}

/** Compiles just the body of a function in whatever is the current context. */
private compileFunctionBody(instance: Function): ExpressionRef[] {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one isn't strictly necessary now that I've gone another route, but who knows what it might be good for in the future.

@MaxGraey
Copy link
Member

MaxGraey commented Feb 1, 2019

LGTM.

Is it possible inline constructors for now?

@dcodeIO
Copy link
Member Author

dcodeIO commented Feb 1, 2019

Inlining constructors seems to be limited in usefulness, except for the Pointer experiment we had a while ago where the constructor essentially eliminates itself when inlined after optimizations. Going to take a look, though, as it shouldn't be too hard to get working again (mostly a matter of looking up actual this instead of assuming index 0).

One special case is if there's no declared constructor, which in turn isn't decorated @inline and inlining these per-se seems to be inefficient if the class has a lot of fields. Currently in favor of not doing this and relying on the user to actually declare a constructor with the decorator if they want it to be inlined.

@MaxGraey
Copy link
Member

MaxGraey commented Feb 1, 2019

Yeah, I mean declare constructor with forced decorator @inline which could be useful for Pointer scenario

@dcodeIO
Copy link
Member Author

dcodeIO commented Feb 1, 2019

Turns out that inlining constructors that didn't return a custom value never worked due to this check preventing it, BUT I think I can at least make the Pointer constructor work again.

@dcodeIO dcodeIO merged commit 2131c51 into master Feb 2, 2019
@dcodeIO
Copy link
Member Author

dcodeIO commented Feb 2, 2019

Merging, continuing from here :)

@dcodeIO dcodeIO deleted the rework-ctors branch March 7, 2019 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants