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

Make regression test work in release builds too. #1042

Merged
merged 2 commits into from
Dec 7, 2019

Conversation

nlewycky
Copy link
Contributor

@nlewycky nlewycky commented Dec 6, 2019

Fix this regression test to detect the bug it was looking for in release builds too.

This bug triggered an assertion failure in debug, and by examining the pre-opt IR, we can check for the bug in release mode too.

…ase builds too.

This bug triggered an assertion failure in debug, and by examining the pre-opt IR, we can check for the bug in release mode too.
@nlewycky nlewycky requested a review from MarkMcCaskey December 6, 2019 00:39
@nlewycky nlewycky changed the title Fix this regression test to detect the bug it was looking for in release builds too. Make regression test work in release builds too. Dec 6, 2019

fn postopt_ir_callback(&mut self, _: &InkwellModule) {}

fn obj_memory_buffer_callback(&mut self, _: &InkwellMemoryBuffer) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

You can provide (empty) defaults in the trait itself. It may also be good to use unimplemented!() here to catch mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't want to use unimplemented!() since it's totally reasonable to implement only one or two of them. Done!

@nlewycky nlewycky requested a review from losfair as a code owner December 6, 2019 20:09
@nlewycky
Copy link
Contributor Author

nlewycky commented Dec 6, 2019

bors r+

bors bot added a commit that referenced this pull request Dec 6, 2019
709: new feature flag: `deterministic` r=MarkMcCaskey a=YaronWittenstein

The motivation for the PR is for introducing a new feature flag called `deterministic`.

When `deterministic` will be enabled (turned-off by default) it'll guarantee deterministic
execution of wasm programs across different hardware/circumstances.

This is critical for Blockchain projects that require execution to be deterministic
in order to reach a consensus of the state transition of each smart-contract transaction.

865: adding tests for `state_creator` of `import_object` r=MarkMcCaskey a=YaronWittenstein

Part of the PR #807 changes was adding support for shared import objects between threads.

https://github.com/wasmerio/wasmer/pull/807/files#diff-d20cb4c5a883566b85be4cc046f45aa9R49

I've added tests/examples on how to create an `import object` with a state_creator
(function or closure)

1042: Make regression test work in release builds too. r=nlewycky a=nlewycky

Fix this regression test to detect the bug it was looking for in release builds too.

This bug triggered an assertion failure in debug, and by examining the pre-opt IR, we can check for the bug in release mode too.


Co-authored-by: Yaron Wittenstein <[email protected]>
Co-authored-by: Yaron Wittenstein <[email protected]>
Co-authored-by: Mark McCaskey <[email protected]>
Co-authored-by: Mark McCaskey <[email protected]>
Co-authored-by: Nick Lewycky <[email protected]>
@bors
Copy link
Contributor

bors bot commented Dec 6, 2019

Build failed (retrying...)

bors bot added a commit that referenced this pull request Dec 6, 2019
865: adding tests for `state_creator` of `import_object` r=MarkMcCaskey a=YaronWittenstein

Part of the PR #807 changes was adding support for shared import objects between threads.

https://github.com/wasmerio/wasmer/pull/807/files#diff-d20cb4c5a883566b85be4cc046f45aa9R49

I've added tests/examples on how to create an `import object` with a state_creator
(function or closure)

1042: Make regression test work in release builds too. r=nlewycky a=nlewycky

Fix this regression test to detect the bug it was looking for in release builds too.

This bug triggered an assertion failure in debug, and by examining the pre-opt IR, we can check for the bug in release mode too.


Co-authored-by: Yaron Wittenstein <[email protected]>
Co-authored-by: Nick Lewycky <[email protected]>
@bors
Copy link
Contributor

bors bot commented Dec 6, 2019

Build failed (retrying...)

bors bot added a commit that referenced this pull request Dec 6, 2019
1042: Make regression test work in release builds too. r=nlewycky a=nlewycky

Fix this regression test to detect the bug it was looking for in release builds too.

This bug triggered an assertion failure in debug, and by examining the pre-opt IR, we can check for the bug in release mode too.


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

bors bot commented Dec 7, 2019

Build succeeded

@bors bors bot merged commit a221f1e into master Dec 7, 2019
@bors bors bot deleted the feature/llvm-backend-tests branch December 7, 2019 00:24
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.

2 participants