On Wed, 4 Jun 2025, 00:53 Michael Welsh Duggan, <mwd_at_[hidden]> wrote:

> Jonathan Wakely <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_gmail.com. 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.
>

I would also mention that it makes is_constructible_v<hh_mm_ss<ud>, ud> lie
when ud is a duration with unsigned rep.



> 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()).
>

Ideally you'd show the changes with <ins> and <del> tags, like:

<del>abs(d)</del><ins>ABS_D</ins>

Otherwise this looks OK.