Skip to content

Fix managed function expression return #1027

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 3 commits into from
Dec 28, 2019
Merged

Fix managed function expression return #1027

merged 3 commits into from
Dec 28, 2019

Conversation

dcodeIO
Copy link
Member

@dcodeIO dcodeIO commented Dec 27, 2019

As reported in #1026 there is an issue when a managed reference is returned from function expressions using just a single expression.

To fix this, I ultimately decided to unify preparation of potentially managed return values by reusing autorelease undo functionality originally implemented for branch unification. As a side-effect this also gets rid of various unnecessary retain/release pairs whenever the undo succeeded.

fixes #1026

@dcodeIO dcodeIO merged commit 747ebde into master Dec 28, 2019
@MaxGraey
Copy link
Member

btw I'm wondering why this

local.tee $0
call $~lib/rt/pure/__retain
local.get $0
call $~lib/rt/pure/__release

not optimized by binaryen's Optimize ARC pass. It seems that remove RC methods for local.set/get pair but not for local.tee/get?

@dcodeIO
Copy link
Member Author

dcodeIO commented Dec 28, 2019

The pass collects retains of the form local.set(X, __retain(...)), so this isn't a full retain pattern it would recognize. Is this present in untouched output or does it happen after other optimizations? (can you link to the fixture?)

@MaxGraey
Copy link
Member

MaxGraey commented Dec 28, 2019

can you link to the fixture?

It already solved in this PR, but I thought it shouldn't be a problem for post-assemblyscript pass:
https://github.com/AssemblyScript/assemblyscript/pull/1027/files#diff-d48956ee38e0493bf00054580270b5d3L7512

I guess we have that patterns in other places buy I'm not sure

@dcodeIO dcodeIO deleted the fix-fexpr-return branch January 1, 2020 14:21
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.

Runtime error when trying to instantiate a class in arrow function
2 participants