[time.cal.year.nonmembers] Avoid narrowing conversion #4184
Conversation
| @@ -4771,7 +4771,7 @@ | |||
| \begin{itemdescr} | |||
| \pnum | |||
| \returns | |||
| \tcode{year\{int\{x\} + y.count()\}}. | |||
| \tcode{year\{int\{x\} + static_cast<int>(y.count())\}}. | |||
There was a problem hiding this comment.
We need a precondition that y.count() is representable by int to avoid UB in the conversion. It might be simplest to switch from \returns: meow to \effects: Equivalent to: \tcode{return meow;} so the precondition is implicit.
There was a problem hiding this comment.
The conversion is never UB. The addition might, but it could be today as well.
There was a problem hiding this comment.
Thanks - I forget that everything's twos-complement now. I should probably file an LWG issue for the wider problem of "Returns: non-well-defined"; I'm reasonably certain we want implied preconditions, but we don't say so.
There was a problem hiding this comment.
Just for grins, the recommendation is to stuff this result into a signed 16 bit integral (short). :-) Rationale: If your civil year number is outside of the range +/-32K, you're hosed anyway. The civil calendar models the solar system with an accuracy of about 1 day in 4K years.
The years::rep type could be a signed integer wider than int.
jwakely
force-pushed
the
cal-narrowing
branch
from
7f133cc
to
03cb6b6
Compare
|
Adding LWG label: while the narrowing check here may be unintentional, it seems to be a normative requirement (whatever it means for the "Returns:" clause to be ill-formed), so this PR seems superficially to be a normative change. I'd like LWG to consider this; if the conclusion is that "Returns:" should never be ill-formed and so it's just an obvious wording error that it might be in this case, then I think we can treat this as editorial. |
|
@jwakely, are you dealing with the LWG issue here? |
|
I already raised it and LWG was OK with fixing the unintended flaw editorially. https://lists.isocpp.org/lib/2020/09/17424.php I should have provided that info in the PR, sorry. |
|
Can we remove the lwg label? As I said above, LWG have already discussed it an agreed that resolving it editorially would be best. |
The years::rep type could be a signed integer wider than int.