-
Notifications
You must be signed in to change notification settings - Fork 94
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
Fix libc++ incompatibility #1463
Conversation
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. but maybe need to check .count()
type with static_cast
return std::chrono::duration_cast<std::chrono::nanoseconds, int64>( | ||
return std::chrono::duration_cast<std::chrono::nanoseconds>( | ||
stop.data_.chrono - start.data_.chrono); |
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.
Just to ensure I got it correctly.
duration_cast signature:
template< class ToDuration, class Rep, class Period >
constexpr ToDuration duration_cast( const std::chrono::duration<Rep,Period>& d );
The issue is from type definition of std::chrono::nanoseconds
std::chrono::duration</* int64 */, std::nano>
standard only require it at least 64 bit signed integer.
Thus, it might be not int64.
The type also affect .count()
return type.
Could you check that the .count()
has static_cast to desired type if it is the case?
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 issue is basically that I used to return int64 before and now return nanoseconds, and I didn't change the function call. It only worked accidentally.
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.
I checked, we only use .count()
in places relevant to this issue where it will be cast to double
, so no problems expected
Kudos, SonarCloud Quality Gate passed!
|
This is an issue detected in conan-io/conan-center-index#21058, which uses a different integer type for
std::chrono::nanosecond