-
Notifications
You must be signed in to change notification settings - Fork 50
Fixed warnings in windows compilation [HZ-5297] #1385
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
Conversation
JackPGreen
left a comment
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.
The changes mostly seem to be tightening types (explicit casting etc).
I don't know which warning(s) are being fixed, but I have no real issue with the changes as presented beyond what I've highlighted.
hazelcast/include/hazelcast/client/impl/metrics/metrics_compressor.h
Outdated
Show resolved
Hide resolved
JackPGreen
left a comment
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.
Re-reviewed (GitHub forced me to add a comment as well)
…AGS [HZ-5297] (hazelcast#1386) Replaced `sizeof(frame_header_type)` with `SIZE_OF_FRAME_LENGTH_AND_FLAGS`. fixes hazelcast#1385 (comment)
…conversions and dll imports.
Co-authored-by: Jack Green <[email protected]>
``` hazelcast-cpp-client\hazelcast\src\hazelcast\client\compact.cpp(3349,58): warning C4146: unary minus operator applied to unsigned type, result still unsigned ``` The original code `-(fp & 1L)` does convert the -1 tu uint64_t implicitly but this causes a compiler warning, instead we explicitly cast it and multiply to get the same result. Regarding the value of -(fp & 1L), this is 0 if least siginificant bit of fp is 0 otherwise it results in negated value of 1 which is all 1 bits uint64_t.
87cc22a to
c2ccfbc
Compare
| #include "hazelcast/util/export.h" | ||
| #include "hazelcast/client/serialization/pimpl/compact/field_descriptor.h" | ||
|
|
||
| #if defined(WIN32) || defined(_WIN32) || defined(WIN64) || defined(_WIN64) |
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.
Shouldn’t be here. Disabled globally already?
(Also occurs elsewhere)
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.
yes, you are right. i do not know how it became like this given the rebasing. I fixed them now in 2 files.
| } // namespace serialization | ||
| } // namespace client | ||
| } // namespace hazelcast | ||
| } // namespace hazelcast |
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.
Nit - change of line ending
Fixed warning in windows compilation. Most warnings were due to type conversions and dll imports.
fixes #1350