Skip to content

Conversation

@drug007
Copy link
Contributor

@drug007 drug007 commented Apr 9, 2018

Casting to byte to get negative sign then cast it to the target type T.

Casting to byte to get negative sign then cast to target type T.
@drug007
Copy link
Contributor Author

drug007 commented Apr 9, 2018

Sorry, I haven't check the repository for issue and PR already available.

@drug007 drug007 changed the title Fixed integral promotion (2.079) Fix integral promotion deprecation (#98) Apr 9, 2018
value = cast(T)header;
} else if (0xe0 <= header && header <= 0xff) {
value = -(cast(T)-header);
value = cast(T)cast(byte)header;
Copy link

@RalphBariz RalphBariz Apr 15, 2018

Choose a reason for hiding this comment

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

#99 take a look at this, I tried that one myself first but x86_64 unittests were broken so I had to rethink it :-D

Since that PR is still not handled you can get that version of msgpack-d here: https://github.com/causal-rt/msgpack-d

Copy link

Choose a reason for hiding this comment

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

this looks good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@RalphBariz I've took. I guess your solution is more general, my one handles the specific case only, but due to this fact is much simpler.
But why x64 unittest fails?

Choose a reason for hiding this comment

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

I also tried to remove the -(-()) double negation and faced broken unittests when running in 64bit. Because of that I reintroduced the -- and made that wild casting stuff.

@wilzbach
Copy link

As this repo is rather inactive, but still used by many people of the D community (i.e. DCD) - should we move it to dlang-community?
Explanation: https://dlang.org/blog/2018/02/17/project-highlight-the-d-community-hub/

@repeatedly
Copy link
Member

#99 is merged. So this PR is closed. Thanks and sorry for late response.

@repeatedly repeatedly closed this May 29, 2018
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.

4 participants