Rerumu / Wasynth

WebAssembly to Lua translation library and tool
https://discord.gg/sgm5YcmgyD
GNU General Public License v3.0
130 stars 18 forks source link

Tests with `assert_trap` not generated for `start` #14

Closed Rerumu closed 2 years ago

Rerumu commented 2 years ago

Due to it being kind of annoying, no code is currently generated for an assert_trap that takes a module argument. The intended behavior here is for its start function to run and trap.

https://github.com/Rerumu/Wasynth/blob/master/dev-test/tests/luajit_translate.rs#L90 https://github.com/Rerumu/Wasynth/blob/master/dev-test/tests/luau_translate.rs#L103

Hexcede commented 2 years ago

Correct me if I'm misunderstanding, but, isn't trapping synonymous with throwing specific errors in luau in then asserting that a section of code trapped would be synonymous with asserting that it fails and the message matches when pcalled?

It should look (roughly) like this in luau I believe:

-- generated trapping code
error(failure)

-- generated assert_trap code
local function assert_trap(module, failure)
    local success, message = pcall(module.startFunction)
    assert(not success and message == failure)
end

If this is the case I'm not sure I understand why it's actually annoying. I feel that I'm likely missing information, possibly on the implementation on the rust side of things.

Hexcede commented 2 years ago

Actually, I've looked a little further, and answered my own question, it's about the rust-side mostly. It is true that asserting traps is pretty much like above but the problem comes down to checking against the start function in particular, which I misunderstood. Working on a PR.

Hexcede commented 2 years ago

I would also like to make a note that WASM's assert_trap takes an additional message to match against but this isn't actually handled in the code even though it probably should be. Took a look, and, write_call_of does not allow inserting additional arguments (would have to mutate or copy the instruction(, and while I would like to try and place support for this I'm not sure the proper solution to adding the extra message parameter to assert_trap from the rust side of things.

From the luau/luajit side of things this is really not hard at all.

Rerumu commented 2 years ago

I found the message argument to not be too useful in this case so I dropped it entirely. Might change in the future when tests are expanded to newer versions of the suite.