Skip to content

fix: Fix mixed up naming "upcast" <-> "downcast" for runtime checks #2423

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 2 commits into from
Aug 15, 2022

Conversation

danaugrs
Copy link
Contributor

@danaugrs danaugrs commented Aug 8, 2022

Fixes #2422.

@danaugrs danaugrs force-pushed the fix-downcast-upcast branch from 1c5aff8 to 1f7bcd8 Compare August 8, 2022 12:37
@JesseCodeBones
Copy link
Contributor

I have no permission to edit, but I can tell you the root cause.
wat file will store the error message with utf8 format, as you changed the error message, you should change its wat as well.
postion: line 41 at debug.wat
new message: (data (i32.const 540) "<\00\00\00\00\00\00\00\00\00\00\00\01\00\00\00&\00\00\00u\00n\00e\00x\00p\00e\00c\00t\00e\00d\00 \00d\00o\00w\00n\00c\00a\00s\00t\00\00\00\00\00\00\00")

position: line 37 at release.wat
new message: (data (i32.const 1576) "\01\00\00\00\"\00\00\00u\00n\00e\00x\00p\00e\00c\00t\00e\00d\00 \00d\00o\00w\00n\00c\00a\00s\00t").

Good luck!

@MaxGraey
Copy link
Member

Just recreate all fixtures via ASC_FEATURES="*" npm run test:compiler -- --create command to make sure

@danaugrs
Copy link
Contributor Author

danaugrs commented Aug 10, 2022

I've regenerated all the fixtures and pushed the changes. Thanks for the help guys!

@MaxGraey MaxGraey changed the title Fix downcast and upcast (were switched) fix: Fix mixed up naming "upcast" <-> "downcast" for runtime checks Aug 10, 2022
@danaugrs
Copy link
Contributor Author

@MaxGraey I don't have write access so I can't merge (just letting you know in case you are waiting for me to do so).

@MaxGraey
Copy link
Member

@danaugrs We have an automatic release process (via github action) so we usually collect and merge a bunch of PRs at once. I'll begin the merging process soon

@MaxGraey MaxGraey merged commit 5faef3a into AssemblyScript:main Aug 15, 2022
@MaxGraey
Copy link
Member

Thanks!

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.

Upcast and downcast are switched
3 participants