Simplify next_weekday and prev_weekday calculus

[teksturi]

It seems that calculating next and previous weekday is over complicated. We can make it simpler and faster. On my test on x86-64 I get following results

Run on (14 X 2496.01 MHz CPU s)
CPU Caches:
  L1 Data 48 KiB (x7)
  L1 Instruction 32 KiB (x7)
  L2 Unified 1280 KiB (x7)
  L3 Unified 24576 KiB (x1)
Load Average: 1.68, 0.75, 0.72
---------------------------------------------------------------------------
Benchmark                                 Time             CPU   Iterations
---------------------------------------------------------------------------
BM_NextWeekday                         5.63 ns         5.63 ns    123274543
BM_NextWeekday_new                     3.85 ns         3.85 ns    182678076
BM_PrevWeekday                         4.70 ns         4.70 ns    146452964
BM_PrevWeekday_new                     3.24 ns         3.24 ns    221581381

This is also safer on situations where this function gets weekday as not enum. Yeah that is unlikely but one more plus for this.

Previously there where suggestion 1 for this as

  auto n = static_cast<std::int_fast8_t>(get_weekday(cd));
  auto x = static_cast<std::int_fast8_t>(wd);
  return cd + (1 + (x + 6 - n) % 7);

But that imo is not so easy to understand than this algorithm. Also it did not do so good performance wise.

On test side I added static_assert just in case someone choose to change order of weekday. Higly unlikely but I like to be safe side.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[devbww]

As you've seen, we have consciously chosen not to do this in the past, and I don't see anything that convinces me to change that decision. Saving 1.5ns on an infrequent operation is not worth it. enum class weekday should just be seen as a set of unique labels, without specific values.

This is also safer on situations where this function gets weekday as not enum.

These functions always get weekday as an enum class, because that is what it is. Anyone constructing enum class weekday values from integers is making assumptions not guaranteed by the API.

I'm more inclined to change the declaration order of enum class weekday, or give its elements random values. Nothing in the current code would need to change because it makes no assumptions about the values.

[teksturi]

As you've seen, we have consciously chosen not to do this in the past, and I don't see anything that convinces me to change that decision. Saving 1.5ns on an infrequent operation is not worth it. enum class weekday should just be seen as a set of unique labels, without specific values.

This is also safer on situations where this function gets weekday as not enum.

These functions always get weekday as an enum class, because that is what it is. Anyone constructing enum class weekday values from integers is making assumptions not guaranteed by the API.

I'm more inclined to change the declaration order of enum class weekday, or give its elements random values. Nothing in the current code would need to change because it makes no assumptions about the values.

Ok. This was just suggestion and this is ok to be closed now.

[devbww]

Thanks for thinking about it in any case, Kari.