Skip to content
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

validation: use correct function arity in stack height validation #343

Merged
merged 2 commits into from
May 26, 2020

Conversation

gumb0
Copy link
Collaborator

@gumb0 gumb0 commented May 26, 2020

Required for #312

Resolves one TODO in parse_expr and 45 validation spec tests (must be the tests checking if function stack has enough results at the end of execution)

@gumb0 gumb0 changed the title Pass function type index to parse_expr validation: use correct function arity in stack height validation May 26, 2020
@gumb0 gumb0 requested review from chfast and axic May 26, 2020 15:25
lib/fizzy/parser.hpp Outdated Show resolved Hide resolved
// TODO: Get function result arity from the function type in Module.
const uint8_t func_result_arity = 0;
control_stack.emplace(Instr::block, func_result_arity, 0); // The function's implicit block.
const auto func_type_idx = module.funcsec[func_idx];
Copy link
Member

Choose a reason for hiding this comment

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

Why not pass in func_type_idx?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was like that originally, I changed it just because func_idx seems to be a more natural parameter to "parse instuctions of a function", might be more useful in the future for something.

@gumb0 gumb0 merged commit 924e02d into master May 26, 2020
@gumb0 gumb0 deleted the parse-expr-func-type branch May 26, 2020 17:11
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