eliaskosunen/scnlib

Potential regression from v1.1.3 to v2.0.2

Closed this issue · 3 comments

Updating the submodules in our application and a parsing case that was working before is no longer working. This is a minimal example, in the original the epoch variable is of type const std::string&:

Before:

std::string epoch = "2024-03-23T09:20:33.576864";

struct {
    int year;
    int nDays;
    int hours;
    int minutes;
    double seconds;
} date;
int month = 0;
int days = 0;
auto res = scn::scan(
    epoch, "{:4}-{:2}-{:2}T{:2}:{:2}:{}",
    date.year, month, days, date.hours, date.minutes, date.seconds
);
if (!res) {
    throw ghoul::RuntimeError(fmt::format("Error parsing epoch '{}'", epoch));
}

and no exception being thrown

Now:

std::string epoch = "2024-03-23T09:20:33.576864";

auto res = scn::scan<int, int, int, int, int, double>(
    epoch, "{:4}-{:2}-{:2}T{:2}:{:2}:{}"
);
if (!res) {
    throw ghoul::RuntimeError(fmt::format("Error parsing epoch '{}'", epoch));
}

ends up in vscan.h:115 with result.error() == "Argument list not exhausted"

Also tried with: auto res = scn::scan<int, int, int, int, int>(epoch, "{:4}-{:2}-{:2}T{:2}:{:2}:") and then a second scn::scan<double>(res->range(), "{}") but the first res is also false.

Little follow up information:
std::string epoch = "2024-03-23T09:20:33.576864"; will fail, but std::string epoch = "2024-03-23T19:20:33.576864"; works correctly.

In the former case I actually end up in
scn::v2::anonymous namespace'::format_handler<1,char>::on_literal_text(const char * begin, const char * end) Line 337withit == 9:20:33.576864andbegin == :{}:{}` which results in a "Unexpected literal character in source". So it seems that the 0 in the source string causes some error in the parsing?

This is all on Visual Studio 17.9.3

image

This seems to be an error in the docs: the default behavior for scanning an integer without a type format specifier is i, and not d. This means that the base of the integer to be scanned is determined by its prefix, and it's not decimal by default. So, something like:

auto res = scn::scan<int>("08", "{}");

is scanned as "0" (octal, as '0' is the octal prefix), with "8" left over.

I think I'll revert the default back to decimal (like it was in v1) in the next major release: it seems to be doing more harm than good. For now, it can be worked around by explicitly stating the base with d:

auto res = scn::scan<int, int, int, int, int, double>(
    epoch, "{:4d}-{:2d}-{:2d}T{:2d}:{:2d}:{}"
);

Thanks!