Skip to content

Conversation

@chfast
Copy link
Collaborator

@chfast chfast commented Oct 5, 2020

No description provided.

@chfast chfast requested review from axic and gumb0 October 5, 2020 07:10
@codecov
Copy link

codecov bot commented Oct 5, 2020

Codecov Report

Merging #571 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #571   +/-   ##
=======================================
  Coverage   98.24%   98.24%           
=======================================
  Files          62       62           
  Lines        9052     9062   +10     
=======================================
+ Hits         8893     8903   +10     
  Misses        159      159           

(func $double (param i32) (result i64)
local.get 0
i64.extend_i32_u
local.get 0
Copy link
Member

Choose a reason for hiding this comment

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

Is the point of this test that the same local is used twice?

EXPECT_THAT(execute(module, 3, {4}), Traps());
}

TEST(execute_call, call_indirect_shared_stack_space)
Copy link
Member

Choose a reason for hiding this comment

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

Seems to be a dup of imported_functions_call_indirect ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Both are simplified variants of it. They can detect an implementation defect in #572, so I keep them as they are.

{
/* wat2wasm
(module
(func $double (param i32) (result i64)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's hard to guess what these tests check... Is i64 result required here? If not, maybe make it i32 for simplicity and more focused test?

@chfast chfast merged commit f61a8f1 into master Oct 5, 2020
@chfast chfast deleted the call_tests branch October 5, 2020 11:01
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.

4 participants