On Jun 3, 2025, at 7:52 PM, Michael Welsh Duggan <mwd_at_[hidden]> wrote:

> Jonathan Wakely <cxx_at_[hidden] <mailto:cxx_at_[hidden]>> writes:
>
>> On Tue, 3 Jun 2025 at 23:24, Jonathan Wakely <cxx_at_[hidden]> wrote:
>>
>>>
>>>
>>> On Tue, 3 Jun 2025 at 21:58, Howard Hinnant via Std-Proposals <
>>> std-proposals_at_[hidden]> wrote:
>>>
>>>> I view this as a bug in the spec. It was not intended to outlaw unsigned
>>>> durations for hh_mm_ss.
>>>>
>>>> Upon experimentation I found that my example implementation does not
>>>> outlaw it, gcc does not, msvc does, and llvm does.
>>>>
>>>> A revised wording might look something like: let abs_d have the value -d
>>>> if is_neg is true, else d. Then replace abs(d) with abs_d.
>>>>
>>>> This change in wording will not break gcc. And it will turn cases that
>>>> don’t compile on MSVC and LLVM into cases that do compile, so I believe the
>>>> “breakage” would be acceptable.
>>>>
>>>
>>> It's no accident that it works with GCC:
>>>
>>> commit r13-5002-ge36e57b032b2d70eaa1294d5921e4fd8ce12a74d <https://gcc.gnu.org/cgit/gcc/commit/?id=e36e57b032b2d70eaa1294d5921e4fd8ce12a74d>
>>> Author: Jonathan Wakely
>>> Date: Wed Jan 4 16:43:51 2023 +0000
>>>
>>> libstdc++: Fix std::chrono::hh_mm_ss with unsigned rep [PR108265 <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108265>]
>>>
>>> libstdc++-v3/ChangeLog:
>>>
>>> PR libstdc++/108265 <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108265>
>>> * include/std/chrono (hh_mm_ss): Do not use chrono::abs if
>>> duration rep is unsigned.
>>> * testsuite/std/time/hh_mm_ss/1.cc: Check unsigned rep.
>>>
>>>
>>> In response to https://gcc.gnu.org/PR108265 reported by one Michael
>>> Duggan :-)
>>>
>>>
>> I think this is a conforming extension (no valid program can detect the
>> difference) and I agree with Howard that it doesn't cause any real
>> breakage. The fact that chrono::hh_mm_ss uses chrono::abs internally is not
>> detectable using SFINAE or concepts, because
>> std::is_constructible_v<hh_mm_ss<seconds>, duration<unsigned>> is true,
>> even for implementations where it doesn't actually compile if you try to
>> construct that.
>>
>> Changing the spec to make this work seems like a defect fix, it wouldn't
>> break anything.
>
> Okay. I have never submitted a DR, so I would like a review of the
> message I propose to send to lwgchair_at_[hidden] <mailto:lwgchair_at_[hidden]>. I removed the TeX
> formatting, but I could re-add it if you think it is best.
>
> The chrono::hh_mm_ss constructor is poorly defined for negative
> durations.
>
> In [time.hms.members], paragraph 3, the current wording for the
> constructor of hh_mm_ss expresses some of its requirements in terms of
> abs(d), which is assumed to be chrono::abs(chrono::duration).
> chrono::abs is not defined, however, for durations with an unsigned
> representation. I believe that this is unintentional.
>
> Proposed wording:
>
> Constructs an object of type hh_mm_ss which represents the Duration d
> with precision precision. Let ABS_D represent -d if is_neg is true and
> d otherwise.
>
> Initializes is_neg with d < Duration::zero().
>
> Initializes h with duration_cast<chrono::hours>(ABS_D).
>
> Initializes m with duration_cast<chrono::minutes>(ABS_D - hours()).
>
> Initializes s with duration_cast<chrono::seconds>(ABS_D - hours() -
> minutes()).
>
> If treat_as_floating_point_v<precision::rep> is true, initializes ss
> with ABS_D - hours() - minutes() - seconds().
>
> Otherwise, initializes ss with duration_cast<precision>(ABS_D(d) -
> hours() - minutes()- seconds()).

LGTM. Thanks much for driving this issue.

Howard