Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<chrono>: ambiguous_local_time's message is slightly incorrect #3648

Closed
JMazurkiewicz opened this issue Apr 10, 2023 · 2 comments · Fixed by #3650
Closed

<chrono>: ambiguous_local_time's message is slightly incorrect #3648

JMazurkiewicz opened this issue Apr 10, 2023 · 2 comments · Fixed by #3650
Labels
bug Something isn't working chrono C++20 chrono fixed Something works now, yay! good first issue Good for newcomers

Comments

@JMazurkiewicz
Copy link
Contributor

JMazurkiewicz commented Apr 10, 2023

Bug description

The message of ambiguous_local_time exception class should be (per [time.zone.exception.ambig]/4):

Effects: Initializes the base class with a sequence of char equivalent to that produced by os.str() initialized as shown below:

ostringstream os;
os << tp << " is ambiguous.  It could be\n"
   << tp << ' ' << i.first.abbrev << " == "
   << tp - i.first.offset << " UTC or\n"
   << tp << ' ' << i.second.abbrev  << " == "
   << tp - i.second.offset  << " UTC";

Repro:

#include <cassert>
#include <chrono>
#include <iostream>
#include <string>

int main() {
  const std::string_view required_start =
      "2016-11-06 01:30:00 is ambiguous.  It could be";

  // Example 1, [time.zone.exception.ambig]/4
  using namespace std::chrono;
  try {
    auto zt = zoned_time{"America/New_York",
                         local_days{Sunday[1] / November / 2016} + 1h + 30min};
  } catch (const ambiguous_local_time &e) {
    std::cout << e.what() << '\n';
    assert(std::string_view{e.what()}.starts_with(required_start));
  }
}

Build:

PS D:\stl-playground> clang -std=c++20 .\test.cpp
PS D:\stl-playground> .\a.exe
2016-11-06 01:30:00 is ambiguous. It could be
2016-11-06 01:30:00 GMT-4 == 2016-11-06 05:30:00 UTC or
2016-11-06 01:30:00 GMT-5 == 2016-11-06 06:30:00 UTC   
Assertion failed: std::string_view{e.what()}.starts_with(required_start), file .\test.cpp, line 17

Summary:

"Good first issue" tips

It should say is ambiguous. It could be (two spaces) in those places:

@StephanTLavavej
Copy link
Member

This is very interesting. It appears as 2 spaces in the WG21-N4944 PDF, copies as 1 space, but is 2 spaces in the original LaTeX:

https://github.com/cplusplus/draft/blob/24d7340146caa11227304b18ed57822f1bd53627/source/time.tex#L9181

It does seem that 2 spaces are mandated here.

When fixing this, please note that in addition to the product code in stl/inc/chrono, there are 2 lines of tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp that will need to be updated.

@StephanTLavavej StephanTLavavej added bug Something isn't working good first issue Good for newcomers chrono C++20 chrono labels Apr 10, 2023
@MattStephanson
Copy link
Contributor

This is very interesting. It appears as 2 spaces in the WG21-N4944 PDF, copies as 1 space, but is 2 spaces in the original LaTeX:

Off-topic, but I couldn't stop myself from going down this rabbit hole. For os << tp << " is ambiguous. It could be\n", the corresponding PDF stream is:

[(os)-525(<<)-525(tp)-525(<<)-525(")-525(is)-525(ambiguous.)-1050(It)-525(could)-525(be\134n")]TJ

The TJ operator takes the preceding array and, if the entry is text, displays it. If the entry is a number, that amount is subtracted from the current position. You can see that most words are separated by a 525-unit space, but between "ambiguous." and "It", there's a single, double-width space.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working chrono C++20 chrono fixed Something works now, yay! good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants