Fail time parsing if input is insufficient to supply all fields

[ecatmur]

(and literals) of the format spec, even if it is sufficient to fully
specify the parsable argument.

For example "23:59" is not a valid input for "%H:%M:%S" (or for "%T").

https://eel.is/c++draft/time.parse#17.sentence-1

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.

[ghost]

All CLA requirements met.

[MattStephanson]

@HowardHinnant (or any interested party), if you have time, could you comment on an underlying issue here?

MSVC STL follows libc++ and libstdc++ in setting eofbit in the following situation (godbolt):

#include <cassert>
#include <iomanip>
#include <sstream>

int main() {
    std::tm t;
    std::istringstream iss("2022");
    iss >> std::get_time(&t, "%Y");
    assert(iss.rdstate() == std::ios_base::eofbit);
    return 0;
}

Personally I find the Standard wording here a bit unclear ([locale.time.get.virtuals]/12: "When s == end evaluates to true after reading a character the function evaluates err |= ios_­base​::​eofbit."), but these three implementations agree. (Edit: note that an earlier part of p12 refers to strptime, which specifies that, by default, the "maximum number of characters scanned" for %Y is 4.)

MSVC STL carries over this behavior for std::chrono::parse, because [time] doesn't mention eofbit anywhere. In contrast, your Date library does not set eofbit after an equivalent parse (godbolt):

#include <cassert>
#include <date/date.h>
#include <sstream>

int main() {
    date::year y;
    std::istringstream iss("2022");
    iss >> date::parse("%Y", y);
    assert(iss.rdstate() == std::ios_base::goodbit);
    return 0;
}

Do you have any opinion about what the behavior here should be or if an LWG issue is needed? Thanks.

FWIW, I like Date's behavior, even though it's inconsistent with get_time, because it's more consistent with the philosophy (as I understand it) that eofbit isn't set until we actually try to read after reaching the end. All the numeric chrono parse fields have a known width, either default or user-specified, so it makes sense to me that it should act more like string s; istringstream{"abcd"} >> setw(4) >> s;, which doesn't set eofbit.

[HowardHinnant]

I like date’s behavior as well, and for the same reason. %Y only looks for four digits and finds them. There’s no need for it to go poking around to see if there’s another character or not. Contrast that with “%5Y” which does succeed, and does set eofbit because it does look for another digit and finds eof. Howard

…

On Feb 7, 2022, at 11:31 PM, Matt Stephanson ***@***.***> wrote: @HowardHinnant (or any interested party), if you have time, could you comment on an underlying issue here? MSVC STL follows libc++ and libstdc++ in setting eofbit in the following situation (godbolt): #include <cassert> # include <iomanip> # include <sstream> int main () { std:: tm t; std::istringstream iss("2022" ); iss >> std::get_time(&t, "%Y" ); assert(iss.rdstate () == std::ios_base::eofbit); return 0 ; } Personally I find the Standard wording here a bit unclear ([locale.time.get.virtuals]/12: "When s == end evaluates to true after reading a character the function evaluates err |= ios_­base​::​eofbit."), but these three implementations agree. MSVC STL carries over this behavior for std::chrono::parse, because [time] doesn't mention eofbit anywhere. In contrast, your Date library does not set eofbit after an equivalent parse (godbolt): #include <cassert> # include <date/date.h> # include <sstream> int main () { date::year y; std::istringstream iss("2022" ); iss >> date::parse("%Y" , y); assert(iss.rdstate () == std::ios_base::goodbit); return 0 ; } Do you have any opinion about what the behavior here should be or if an LWG issue is needed? Thanks. FWIW, I like Date's behavior, even though it's inconsistent with get_time, because it's more consistent with the philosophy (as I understand it) that eofbit isn't set until we actually try to read after reaching the end. All the numeric chrono parse fields have a known width, either default or user-specified, so it makes sense to me that it should act more like string s; istringstream{"abcd"} >> setw(4) >> s;, which doesn't set eofbit. — Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you were mentioned.

[StephanTLavavej]

@MattStephanson @HowardHinnant Thanks for the info - that does seem worthy of an LWG issue, especially if we want microsoft/STL, libstdc++, and libc++ to all change to follow Date's behavior.

[MattStephanson]

Looks good to me! I have just one small style nitpick.

[StephanTLavavej]

Thanks, this looks great! Just a couple of minor style questions/issues.

[barcharcraz]

Looks correct to me.

[StephanTLavavej]

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

[StephanTLavavej]

Thanks for fixing this bug in one of C++20's biggest features, and congratulations on your first microsoft/STL commit! 😻 🎉 🚀

This is expected to ship in VS 2022 17.3 Preview 1.