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

Standard exception types for singlepass backend. #1129

Merged
merged 19 commits into from
Jan 20, 2020

Conversation

losfair
Copy link
Contributor

@losfair losfair commented Jan 9, 2020

No description provided.

@losfair
Copy link
Contributor Author

losfair commented Jan 9, 2020

bors try

bors bot added a commit that referenced this pull request Jan 9, 2020
@bors
Copy link
Contributor

bors bot commented Jan 9, 2020

try

Build succeeded

Copy link
Contributor

@Hywan Hywan left a comment

Choose a reason for hiding this comment

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

Thanks for adding more comments in the code!

The code looks good to me.

@@ -208,6 +209,8 @@ impl std::fmt::Display for RuntimeError {
write!(f, "\"{}\"", s)
} else if let Some(s) = data.downcast_ref::<&str>() {
write!(f, "\"{}\"", s)
} else if let Some(exc_code) = data.downcast_ref::<ExceptionCode>() {
write!(f, "\"{:?}\"", exc_code)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should write it is an exception code, so that the user has more insight than the variant name. My suggestion:

write!(f, "exception code \"{:?}\"", exc_code)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Fixed and now the error message looks like Error: RuntimeError: Caught exception of type "Arithmetic"..

@Hywan Hywan added 🎉 enhancement New feature! 📦 lib-compiler-singlepass About wasmer-compiler-singlepass labels Jan 13, 2020
@Hywan Hywan self-assigned this Jan 13, 2020
Copy link
Contributor

@nlewycky nlewycky left a comment

Choose a reason for hiding this comment

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

This looks really good. Thank you!!

The goal was to fix some of the spectests that are excluded from singlepass due to not failing "the right way". I don't see any updates to excludes.txt. Are those fixed?

@losfair
Copy link
Contributor Author

losfair commented Jan 15, 2020

@nlewycky I just updated excludes.txt and removed most singlepass excludes., but there are still a few failures related to linking.wast. I'm not really sure what's going on there.

By the way, I think it would be good to refactor RuntimeError and remove the Any box (and make ExceptionCode a variant + remove plain strings) - maybe in a future PR.

@syrusakbary
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request Jan 16, 2020
1129: Standard exception types for singlepass backend. r=syrusakbary a=losfair



Co-authored-by: losfair <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jan 16, 2020

Build failed

@losfair
Copy link
Contributor Author

losfair commented Jan 20, 2020

bors r+

bors bot added a commit that referenced this pull request Jan 20, 2020
1129: Standard exception types for singlepass backend. r=losfair a=losfair



Co-authored-by: losfair <[email protected]>
@losfair
Copy link
Contributor Author

losfair commented Jan 20, 2020

bors r-

@bors
Copy link
Contributor

bors bot commented Jan 20, 2020

Canceled

@losfair
Copy link
Contributor Author

losfair commented Jan 20, 2020

bors r+

bors bot added a commit that referenced this pull request Jan 20, 2020
1129: Standard exception types for singlepass backend. r=losfair a=losfair



Co-authored-by: losfair <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jan 20, 2020

Build succeeded

@bors bors bot merged commit 1b5d9f2 into master Jan 20, 2020
@bors bors bot deleted the feature/unified-exceptions branch January 20, 2020 18:47
@lexfrl
Copy link

lexfrl commented Feb 5, 2020

Great job! I have a small question: why these traps are RuntimeError::Error? Shouldn't it be a RuntimeError::Trap?

bors bot added a commit that referenced this pull request Feb 10, 2020
1192: Use `ExceptionCode` for error representation. r=losfair a=losfair

Extends #1129 to all backends.

Co-authored-by: losfair <[email protected]>
bors bot added a commit that referenced this pull request Feb 10, 2020
1192: Use `ExceptionCode` for error representation. r=losfair a=losfair

Extends #1129 to all backends.

Co-authored-by: losfair <[email protected]>
bors bot added a commit that referenced this pull request Feb 10, 2020
1192: Use `ExceptionCode` for error representation. r=losfair a=losfair

Extends #1129 to all backends.

Co-authored-by: losfair <[email protected]>
bors bot added a commit that referenced this pull request Feb 11, 2020
1192: Use `ExceptionCode` for error representation. r=losfair a=losfair

Extends #1129 to all backends.

Co-authored-by: losfair <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 enhancement New feature! 📦 lib-compiler-singlepass About wasmer-compiler-singlepass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants