Provide Try{Into|From}Ctx impls for fixed byte array #93
Conversation
|
I just pushed a new change to this PR - which tries to fix a few merge conflicts. @m4b - I do not think the macro rules should be changed. @Frostie314159 - did you intend to change those? |
There was a problem hiding this comment.
@Frostie314159 i left a few feedback comments and questions, let me know what you think
| @@ -1,3 +1,4 @@ | |||
| #![allow(clippy::disallowed_names)] | |||
There was a problem hiding this comment.
i'm not sure this is needed.
| @@ -136,6 +137,11 @@ impl<'a> Segments<'a> { | |||
| Ok(sections) | |||
| } | |||
| } | |||
| impl<'a> Default for Segments<'a> { | |||
There was a problem hiding this comment.
I am not a big fan of having identical default & new -- if anything, maybe better to have the default(), and remove new()?
| @@ -568,12 +570,12 @@ macro_rules! from_ctx_float_impl { | |||
| &mut data as *mut signed_to_unsigned!($typ) as *mut u8, | |||
| $size, | |||
| ); | |||
| transmute(if le.is_little() { | |||
There was a problem hiding this comment.
is there a need for another transmute here?
| unsafe { | ||
| assert!($dst.len() >= $size); | ||
| let bytes = transmute::<$typ, [u8; $size]>(if $endian.is_little() { |
There was a problem hiding this comment.
is there a need for this transmute here?
|
I'm sorry I'm getting very confused with what's going on with all these PRs; I think at this stage we have to have the simple PR with a single commit that just adds the impls for fixed byte arrays; I don't want to review more diffs with unrelated changes, clippy changes, etc. |
|
@m4b agree - I also proposed this in #86. I created this PR only to help @Frostie314159 to see what their PR actually changes compared with the main branch. There is no need to merge it. |
|
i see, thank you @nyurik , i'm going to go to sleep now, it's late for me, thanks for both of your hard work and PRs, much appreciated :) |
|
Thanks for the help! |
m4b
force-pushed
the
master
branch
2 times, most recently
from
18d11bc
to
d369188
Compare
|
this is done in another pr |
This is a squashed #86. I do not think this should be merged as is because it has a few reverts and a few other unrelated things.