tomaszkam / date

A date and time library based on the C++11/14/17 <chrono> header
Other
0 stars 0 forks source link

[EXT] Do survey for move-only TimeZonePtr #33

Closed tomaszkam closed 5 years ago

tomaszkam commented 5 years ago

Original comment:

Do survey for move-only TimeZonePtr

tomaszkam commented 5 years ago

I have assumed that this would be post C++20, extension but are not really sure now.

tomaszkam commented 5 years ago

@HowardHinnant: Supporting this feature seems to be relatively easy, as it requires two things: 1) Making sure that TimeZonePtr is moved zone_ member 2) Adding zoned_time(zoned_time<Duration2, TimeZonePtr>&&) constructor that will move time zone.

[ Note: The (string_view/TimeZonePtr, zoned_time<Duration, TimeZonePtr> const&) constructor does need to have zoned_time<Duration, TimeZonePtr> version, as they do not use zone. ]

We already have 1) in place (see https://github.com/tomaszkam/date/blob/master/include/date/tz.h#L1367), however, we miss the move converting constructor (2). I think that this can be handled as post-C++20 material. Do you agree?

HowardHinnant commented 5 years ago

The intent of the original design was to support a move-only TimeZonePtr. This is evidenced by the std::move sprinkled throughout the spec. This looks like a simple oversight that we still have time to fix for C++20.

But I also think it could be done post C++20. It would be adding functionality, not restricting it, so nothing would break.

tomaszkam commented 5 years ago

I realized that there are more things that we would need to add, like the ability to move-out move-only zone from zoned_timed temporary, i.e. change the get_time_zone to have two overloads:

TimeZonePtr get_time_zone() &&;
TimeZonePtr get_time_zone() const&;

I think that the we are fine for C++20, as we support constructing zoned_time with move-only TimeZonePtr (e.g. using unique_ptr).

tomaszkam commented 5 years ago

But do you agree that this need to go through LEWG?

HowardHinnant commented 5 years ago
  1. I purposefully did not overload get_time_zone(). This would be equivalent to letting z == nullptr in zoned_time(TimeZonePtr z), and that would add too much overhead and uncertainty to each accessor.

  2. Any minor tweaks like this, I'm happy to leave the procedure up to Titus and Marshall. What often happens is Marshall tells Titus what the LWG thinks and asks if he wants the LEWG to review it. Titus says no. But if the tweak gets major enough, Titus will (and should) say yes. In any event, that decision is theirs and not ours.

HowardHinnant commented 5 years ago

Oh! <slaps forehead> And that is why I didn't add zoned_time(zoned_time<Duration2, TimeZonePtr>&&)!

I never move from the TimeZonePtr member in the zoned_time. That is simply functionality that the move-only TimeZonePtr author doesn't get.

tomaszkam commented 5 years ago

Yeah, we also do not have move-constructor from time_zone (as the copy constructor is user-declared). But that implies that zoned_time with move-only TimeZonePtr is not very usable. I mean you can create one, but you cannot move it around, put it into vector, or do other reasonable things with it. Example:

zoned_time<seconds, MoveOnlyTimeZone> func();
{
   zoned_time<seconds, MoveOnlyTimeZone> local(...);
   return local; //ill-formed
}

If we want reasonable support for move-only TimeZonePtr, we need to allow a zoned_time with z == nullptr in moved-from state. I think we should get some user request for this feature before adding it.

Closing as EXT post-C++20.