Skip to content

Fixed demangles an assemblyscript module #363

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 4 commits into from
Dec 7, 2018

Conversation

kazupon
Copy link
Contributor

@kazupon kazupon commented Dec 7, 2018

No description provided.

@dcodeIO
Copy link
Member

dcodeIO commented Dec 7, 2018

These look like good changes to me. Going to break code that uses explicit this arguments instead of Ctor.wrap of course. @MaxGraey wdyt?

@MaxGraey
Copy link
Member

MaxGraey commented Dec 7, 2018

LGTM

@@ -265,6 +265,7 @@ function demangle(exports, baseModule) {
let classElem = curr[className];
if (typeof classElem === "undefined" || !classElem.prototype) {
let ctor = function(...args) {
args.unshift(null); // pass null pointer for class constructor of AssemblyScript
return ctor.wrap(ctor.prototype.constructor(...args));
Copy link
Member

Choose a reason for hiding this comment

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

Would be a bit more efficient to write as return ctor.wrap(ctor.prototype.constructor(0, ...args)); I think

value: function (...args) {
args.unshift(this.this);
setargc(args.length);
return elem(...args);
Copy link
Member

Choose a reason for hiding this comment

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

Similar: return elem(this.this, ...args);

Copy link
Member

@MaxGraey MaxGraey Dec 7, 2018

Choose a reason for hiding this comment

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

@dcodeIO this.this looks strange. May be better rename it to self or value for object's value in wrap function?

Copy link
Member

Choose a reason for hiding this comment

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

I only used .this as the property name because is is less likely to conflict with a class's own fields and methods. Could still conflict, though.

Copy link
Member

Choose a reason for hiding this comment

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

got it. What about using Symbol?

Copy link
Member

Choose a reason for hiding this comment

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

Good call, done in 86c084a

@kazupon
Copy link
Contributor Author

kazupon commented Dec 7, 2018

done!

@dcodeIO dcodeIO merged commit 429435c into AssemblyScript:master Dec 7, 2018
@kazupon
Copy link
Contributor Author

kazupon commented Dec 7, 2018

@dcodeIO Thanks! :)

@kazupon kazupon deleted the fix/loader branch December 7, 2018 16:32
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.

3 participants