Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

Fix MSVC test failures. #269

Merged
merged 12 commits into from
Apr 26, 2022
Merged

Fix MSVC test failures. #269

merged 12 commits into from
Apr 26, 2022

Conversation

wmaxey
Copy link
Member

@wmaxey wmaxey commented Apr 22, 2022

This commit chain will include fixes to address failures detected across several versions of MSVC. I've used the compilers listed below that I believe are major releases for MSVC.

MSVC versions that are clean:

  • 14.12
  • 14.13
  • 14.16
  • 14.22
  • 14.24
  • 14.27
  • 14.29
  • 14.30

@wmaxey wmaxey added this to the 1.8.1 milestone Apr 22, 2022
@wmaxey wmaxey requested a review from griwes April 22, 2022 23:31
@wmaxey wmaxey marked this pull request as ready for review April 23, 2022 02:01
@@ -19,6 +19,11 @@

#include "test_macros.h"

#if defined(_MSC_VER)
# pragma warning( disable: 4307 )
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add comments to these, similarly how the 4244 disables have comments?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added comments.

@@ -2534,7 +2534,7 @@ year_month_day::__from_days(days __d) noexcept
const unsigned __doy = __doe - (365 * __yoe + __yoe/4 - __yoe/100); // [0, 365]
const unsigned __mp = (5 * __doy + 2)/153; // [0, 11]
const unsigned __dy = __doy - (153 * __mp + 2)/5 + 1; // [1, 31]
const unsigned __mth = __mp + (__mp < 10 ? 3 : -9); // [1, 12]
const unsigned __mth = __mp + static_cast<unsigned>(__mp < 10 ? 3 : -9); // [1, 12]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Re-align the trailing comment with the 9nes in the previous lines.

Copy link
Member Author

Choose a reason for hiding this comment

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

Realigned.

const unsigned __doy = (153 * (__mth + (__mth > 2 ? -3 : 9)) + 2) / 5 + __dy-1; // [0, 365]
const unsigned __doe = __yoe * 365 + __yoe/4 - __yoe/100 + __doy; // [0, 146096]
const unsigned __yoe = static_cast<unsigned>(__yr - __era * 400); // [0, 399]
const unsigned __doy = static_cast<unsigned>((153 * (__mth + static_cast<unsigned>(__mth > 2 ? -3 : 9)) + 2) / 5 + __dy-1); // [0, 365]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is the outer static_cast needed here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Two problems mainly. __mth + negative signed integer is a warning. The assignment ends up being a demotion as well after xy / 5 + __dy-1

@wmaxey wmaxey merged commit b9f21ee into main Apr 26, 2022
@wmaxey wmaxey deleted the bugfix/msvc_test_failures branch April 26, 2022 22:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants