Stream: cranelift

Topic: semver-compatible wasmparser 0.63.1 changes error messages


view this post on Zulip Chris Fallin (Oct 07 2020 at 17:06):

Hi @Alex Crichton et al -- quick note that wasmparser 0.63.1 is potentially an issue for Firefox as it changes error messages slightly, but is semver-compatible with cranelift-codegen's "0.63" dep; so when we develop locally (not against the wasmparser version that is vendored into the mozilla-central repo) we have non-passing tests that pass otherwise

view this post on Zulip Chris Fallin (Oct 07 2020 at 17:07):

Could we have a policy of semver-bumping when we change error messages?

view this post on Zulip Alex Crichton (Oct 07 2020 at 18:00):

oh hm, so typically semver-bumps are generally about APIs as in "your code keeps compiling and probably keeps working"

view this post on Zulip Alex Crichton (Oct 07 2020 at 18:00):

"keeps working" is sorta vague, though, and typically things like error messages don't fall under that category

view this post on Zulip Alex Crichton (Oct 07 2020 at 18:00):

if it's causing issues though I can yank it and publish as 0.64.0

view this post on Zulip Alex Crichton (Oct 07 2020 at 18:00):

ideally making error messages better though wouldn't cause too much process to get engaged

view this post on Zulip Chris Fallin (Oct 07 2020 at 18:04):

I don't think a yank is necessary, probably, it'll just need some updating in the tests next time someone vendors Cranelift

view this post on Zulip Chris Fallin (Oct 07 2020 at 18:05):

(... which may be me, today or tomorrow)

view this post on Zulip Chris Fallin (Oct 07 2020 at 18:06):

in any case I don't want SM to be a drag on improving error messages :-) ideally if things are somewhat stable then this will mostly be a non-issue. And longer-term we should find a better strategy for asserting specific errors

view this post on Zulip fitzgen (he/him) (Oct 08 2020 at 15:59):

in build.rs-generated tests we only check that the spec test expected error messages are a prefix of the actual error messages, so we have some wiggle room. is SM testing full messages? is there a way we could relax it a bit like we do in build.rs?

view this post on Zulip Chris Fallin (Oct 08 2020 at 15:59):

Yep, it's matching on full messages, or at least more of the text than the prefix (e.g., it wants to assert specific types in type-mismatch messages)

view this post on Zulip Chris Fallin (Oct 08 2020 at 16:00):

I suppose it could be relaxed, but that's more of a question for Lars over in SpiderMonkey-land (we can certainly bring it up!)

view this post on Zulip fitzgen (he/him) (Oct 08 2020 at 16:00):

yeah it's a trade off


Last updated: Oct 23 2024 at 20:03 UTC