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
[time.cal] improve std::chrono::month
operator+
's description of arithmetic mod 12
#1974
Comments
This seems to be related to the pending motions. There is no class "month" in the current working draft. |
The "Euclidean division" description is repeated in the description of |
I'm kind of fond of the specification in terms of Euclidean division. I believe it says precisely what we want it to say. However your point about overflow is well taken. One possible solution is to add to [time.cal.general] something along the lines of:
That's probably not exactly right. But something in that direction... |
@HowardHinnant, what exactly do you fear will be lost when we talk about "reduced modulo y" instead? That seems to be fully equivalent, and one could argue that explicitly specifying "Euclidian division" might actually be overspecification (it's purporting to prescribe an algorithm, not a result.) |
std::chrono::month
operator+
's description of arithmetic mod 12std::chrono::month
operator+
's description of arithmetic mod 12
I see two problems:
I'm using this terminology for modulo and it must give a result in the range [0, 11] for negative and non-negative |
@HowardHinnant, thanks for your thoughts. I believe we very carefully use English, not C++ code, to describe the modulo operation here, which is thus clearly highlighted as a mathematical operation, not a C++ operation. All we're doing is getting the canonical representative for the equivalence class, and we clearly say these are defined to be in [1,12] in our case. |
I'm happy for the LWG to explore alternative wording, and to help review said wording. The most important thing for me is that vendors know what to provide and that customers understand what they are getting. I would prefer that this change not be done editorially, but through a LWG issue so that it benefits from a full review of the LWG. |
Perhaps something like:
I think that should completely avoid the "number between 0 and 11" interpretation. Just some food for thought for the LWG discussion. |
Perhaps we could just say:
Returns: a
month
withm_
equal to the sum ofunsigned{x}
andy.count()
, reduced modulo 12 to an integer in[1, 12]
.or something like that, rather than defining an auxiliary
modulo(n, 12)
function and going on an excursion to a definition of Euclidean division.One problem: this is not entirely editorial, as the new wording does not admit any undefined behavior, whereas the description in the existing wording results in UB when
long long
andmonths
' representation type are the same size and themonths
value is nearlyLLONG_MAX
. The undefined behavior here does not appear to be especially useful; implementations can easily produce the correct result by reducing themonths
value modulo 12 first, if they chose a representation type formonths
that is the same size aslong long
.The text was updated successfully, but these errors were encountered: