Skip to content

Conversation

@christoph-cullmann
Copy link
Contributor

long is only 32-bit on 32-bit systems and on 64-bit Windows ensure we have consistent behavior

long is only 32-bit on 32-bit systems and on 64-bit Windows
ensure we have consistent behavior
@CLAassistant
Copy link

CLAassistant commented Feb 13, 2025

CLA assistant check
All committers have signed the CLA.

@christoph-cullmann
Copy link
Contributor Author

See coin-or/Cbc#690

@jjhforrest

@jhmgoossens
Copy link
Contributor

The two Windows CI builds for "arch: x86_64" both fail with "error: 'int64_t' does not name a type;"
This seems to be caused by the stdint.h not getting automatically included via COINUTILS_HAS_STDINT_H.
Can this be fixed before merging?

@tkralphs tkralphs merged commit 328760e into coin-or:master Feb 17, 2025
13 checks passed
@christoph-cullmann
Copy link
Contributor Author

Thanks for fixing the missing header!

@jhmgoossens
Copy link
Contributor

@christoph-cullmann A little late, but can you resolve also the compiler Warnings about int64_t?

numerator_ = val;
CoinRational.cpp(32,18): warning C4244: '=': conversion from 'double' to 'int64_t', possible loss of data
denominator_ = 1.0;
CoinRational.cpp(33,20): warning C4244: '=': conversion from 'double' to 'int64_t', possible loss of data
numerator_ += std::abs(intpart) * denominator_;
CoinRational.cpp(86,35): warning C4244: '+=': conversion from 'double' to 'int64_t', possible loss of data

I guess an explicit cast to int64_t would be sufficient? or a round function?

These compiler warnings (and all others) are not shown in the Linux and Windows build logs using coinbrew, but show in the Windows MSVS build logs. At least instead of 254 such warnings we'd have a few less.

@christoph-cullmann
Copy link
Contributor Author

Mm, what is the right fix for that? Before there was the same data loss, even more, with the assignment to 32 bit or 64 bit ints. Does one just want a cast? Or does one want to die as it is erroneous if out of range?

@jhmgoossens
Copy link
Contributor

In all three cases, the actual values are integer anyway. For example, if (floor(val)==val) .... Therefore, a simple explicit cast is OK, like numerator_ = (int64_t) val;. I also added some tests in #246. The bigger remaining question for me is why the Cgl tests failed, but that's for Cgl.

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