-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Fix std::chrono/date conflict for mac builds with C++20 #22138
base: main
Are you sure you want to change the base?
Conversation
…e of xcode/targets.
We may also consider using absl::date, then we can totally get rid of the third-party date library, regardless C++ standard version. // Construct an absl::Time from the system clock.
absl::Time t1 = absl::Now();
// When formatting a time, a time zone should be passed.
absl::TimeZone utc = absl::UTCTimeZone();
std::cout << absl::FormatTime(t1, utc); |
What's the difference between absl::date and the date library from Howard Hinnant that we use? Both are third-party aren't they? I would have assumed the latter is closer to what is in the C++20 standard given Howard Hinnant is on the C++ standards committee. |
Howard Hinnant is out of maintenance. On the other hand, absl::date is not deprecated, and it is already in the list of ORT's dependencies. Anyway we only use it in very a few places. The change would be local. |
// C++20 has operator<< in std::chrono for Timestamp type but mac builds need additional checks | ||
// to ensure usage is valid. | ||
// TODO: As we enable C++20 on other platforms we may need similar checks. | ||
#define _USE_CXX20_STD_CHRONO __cplusplus >= 202002L |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: the macro name looks like a compiler internal macro.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a temporary define which is undef'd as soon as used. Is there a better format to use?
You need to run clang-format on the changed files.
I assume that's because it was added to the C++20 standard: https://github.com/HowardHinnant/date#standardization Ideally we'd only use one date library. If there's other functionality from absl::date we need I'm fine with that being the choice (assuming no significant binary size impact). But if this is all short term and we expect the C++20 features to remove the need for a third part date library I'd leave things as-is for now. |
Description
Fix usage of c++ std::chrono::operator<< in mac builds for wider range of xcode/targets.
Motivation and Context
#21033