LWG-3536 Should chrono::from_stream assign zero to duration for failure?

[StephanTLavavej]

Noticed by @MattStephanson and investigated by @mnatsuhara.

WG21-N4878 [time.duration.io]/3:

template<class charT, class traits, class Rep, class Period, class Alloc = allocator<charT>>
  basic_istream<charT, traits>&
    from_stream(basic_istream<charT, traits>& is, const charT* fmt,
                duration<Rep, Period>& d,
                basic_string<charT, traits, Alloc>* abbrev = nullptr,
                minutes* offset = nullptr);

Effects: Attempts to parse the input stream is into the duration d using the format flags given in the NTCTS fmt as specified in [time.parse]. If the parse parses everything specified by the parsing format flags without error, and yet none of the flags impacts a duration, d will be assigned a zero value. If %Z is used and successfully parsed, that value will be assigned to *abbrev if abbrev is non-null. If %z (or a modified variant) is used and successfully parsed, that value will be assigned to *offset if offset is non-null.

[time.parse]/11:

If the type being parsed cannot represent the information that the format flag refers to, is.setstate(ios_base::failbit) is called.
[Example 1: A duration cannot represent a weekday. -- end example]
However, if a flag refers to a "time of day" (e.g. %H, %I, %p, etc.), then a specialization of duration is parsed as the time of day elapsed since midnight.

[time.parse]/12:

If the from_stream overload fails to parse everything specified by the format string, or if insufficient information is parsed to specify a complete duration, time point, or calendrical data structure, setstate(ios_base::failbit) is called on the basic_istream.

What happens for the duration overload of from_stream()? My interpretation of the Standardese:

Format	Stream	Result	Rationale
"%j"	"10"	Success, days{10}
"%j"	"meow"	failbit, unknown
"%a"	"Saturday"	failbit, duration::zero()	[time.parse]/11 "cannot represent", [time.duration.io]/3 "none of the flags impacts a duration"
"%a"	"Caturday"	failbit, unknown
"literal"	"literal"	failbit, duration::zero()	[time.parse]/12 "insufficient information", [time.duration.io]/3 "none of the flags impacts a duration"
"literal"	"meow"	failbit, unknown

When the parse simply fails, should the duration be set to zero, or should it be unchanged? Nothing seems to address this specific case.

On one hand, it seems that there's no Standardese telling us to assign zero, so we shouldn't. On the other hand, it seems very strange for "%a" parsing "Saturday" to set failbit and assign zero, while "%a" parsing "Caturday" sets failbit and leaves the duration unchanged.

The other from_stream() overloads all clearly address this, e.g. [time.cal.wd.nonmembers]/7 "If the parse fails to decode a valid weekday, is.setstate(ios_base::failbit) is called and wd is not modified."

[CaseyCarter]

@HowardHinnant any opinion on this issue? Should the standard be clarified?

[HowardHinnant]

This looks like a bug in the standard to me, due to two errors on my part.

I believe that the standard should clearly say that if failbit is set, the parsed variable (duration, time_point, whatever) is not modified. I mistakenly believed that the definition of unformatted input function covered this behavior. But after review, I don't believe it does. Instead each extraction operator seems to say this separately.

I also at first did not have my example implementation coded to leave the duration unchanged. So that's how the wording got in in the first place. Here's the commit where I fixed my implemenation:
HowardHinnant/date@d53db7a. And I failed to propagate that fix into the proposal/standard.

I believe this is worthy of an issue that suggests that this sentence be removed. And that this sentence be appended to:

, and the argument duration, time point, or calendrical data structure shall not be modified.

[MattStephanson]

That sound good, but it makes the wording a bit inconsistent. The duration specialization would be covered by [time.parse]/12, while all the others would be explicitly (and redundently) covered by the individual from_stream specifications, e.g., [time.cal.day.nonmembers]/8. I think it would be more consistent to replace
the sentence in [time.duration.io]/3 with

If the parse fails to decode a valid duration, is.setstate(ios_­base​::​failbit) is called and d is not modified.

Alternatively, you could add Howard's proposed wording to [time.parse]/12 and remove the "If the parse fails to decode..." from the individual from_stream specifications. I can write it up either way, unless someone else would prefer to do it.

[HowardHinnant]

I would really appreciate you writing it up @MattStephanson! And I'm fine with whichever wording you prefer. I had forgotten that we already had specific wording elsewhere.

[CaseyCarter]

I can write it up either way, unless someone else would prefer to do it.

+1 That would be awesome, @MattStephanson. If you do report this to LWGChair, please report back here when the issue number is assigned so we can keep everything tied together.

[CaseyCarter]

Oh, and thanks a std::mega for your help, @HowardHinnant!

[MattStephanson]

Issue 3536, my first!