-
Notifications
You must be signed in to change notification settings - Fork 293
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
refactor: impl ExtendInto and WrapInto for Self #541
Conversation
What is the advantage of having ExtendInto compared to a simple From implementation? |
BENCHMARKS
|
Codecov Report
@@ Coverage Diff @@
## master #541 +/- ##
==========================================
- Coverage 79.79% 79.73% -0.06%
==========================================
Files 75 75
Lines 6309 6303 -6
==========================================
- Hits 5034 5026 -8
- Misses 1275 1277 +2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Maybe I think that these trait implementations are only for one-to-one correspondence with wasm instructions, so I do so reasonably. Just like |
@Robbepop BTW, such as these impls are not used, since wasmi do not directly use f32/f64. impl_extend_into!(i64, f64);
impl_extend_into!(u64, f64);
impl_extend_into!(f32, f64); |
Yeah I just saw that. Was looking at this PR from mobile and didn't get the code on the small format. |
I think it makes sense to remove those impls. Generally there never was a clear design behind when to use |
Renamed the PR since this does not implement any new feature but acts more like a refactoring of code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I think this kind of generalization of load/store operations is a step forward. |
No description provided.