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

Please use std::chrono::parse instead of date::parse #67

Merged
merged 2 commits into from
May 24, 2024

Conversation

toge
Copy link
Contributor

@toge toge commented May 11, 2024

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • CHANGELOG.md has been updated (for bug fixes / features / docs)

What kind of change does this PR introduce?

Several compiler(gcc14, latest MSVC) don't support "date" library on C++20 or later.

compilation errors on gcc14 in C++20.

src/reduct/client.cc: In member function ‘virtual reduct::Resultreduct::IClient::ServerInfo reduct::Client::GetInfo() const’:
src/reduct/client.cc:60:24: error: no matching function for call to ‘parse(const char [7], reduct::IClient::Time&)’
60 | date::parse("%FT%TZ", server_info.license->expiry_date);
| ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from src/reduct/client.cc:5:
include/date/date.h:8086:1: note: candidate: ‘template<class Parsable, class CharT, class Traits, class Alloc> decltype ((date::from_stream(declval<std::basic_istream<_CharT, _Traits>&>(), format.c_str(), tp), date::parse_manip<Parsable, CharT, Traits, Alloc>{format, tp})) date::parse(const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&, Parsable&)’
8086 | parse(const std::basic_string<CharT, Traits, Alloc>& format, Parsable& tp)
| ^~~~~
include/date/date.h:8086:1: note: template argument deduction/substitution failed:
src/reduct/client.cc:60:24: note: mismatched types ‘const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>’ and ‘const char [7]’
60 | date::parse("%FT%TZ", server_info.license->expiry_date);
| ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/date/date.h:8097:1: note: candidate: ‘template<class Parsable, class CharT, class Traits, class Alloc> decltype ((date::from_stream(declval<std::basic_istream<_CharT, _Traits>&>(), format.c_str(), tp, (& abbrev)), date::parse_manip<Parsable, CharT, Traits, Alloc>{format, tp, (& abbrev)})) date::parse(const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&, Parsable&, std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&)’
8097 | parse(const std::basic_string<CharT, Traits, Alloc>& format, Parsable& tp,
| ^~~~~
include/date/date.h:8097:1: note: candidate expects 3 arguments, 2 provided
include/date/date.h:8109:1: note: candidate: ‘template<class Parsable, class CharT, class Traits, class Alloc> decltype ((date::from_stream(declval<std::basic_istream<_CharT, _Traits>&>(), format.c_str(), tp, declval<std::__cxx11::basic_string<_CharT, _Traits, _Alloc>>(), (& offset)), date::parse_manip<Parsable, CharT, Traits, Alloc>{format, tp, nullptr, (& offset)})) date::parse(const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&, Parsable&, std::chrono::minutes&)’
8109 | parse(const std::basic_string<CharT, Traits, Alloc>& format, Parsable& tp,
| ^~~~~
include/date/date.h:8109:1: note: candidate expects 3 arguments, 2 provided
include/date/date.h:8123:1: note: candidate: ‘template<class Parsable, class CharT, class Traits, class Alloc> decltype ((date::from_stream(declval<std::basic_istream<_CharT, _Traits>&>(), format.c_str(), tp, (& abbrev), (& offset)), date::parse_manip<Parsable, CharT, Traits, Alloc>{format, tp, (& abbrev), (& offset)})) date::parse(const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&, Parsable&, std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&, std::chrono::minutes&)’
8123 | parse(const std::basic_string<CharT, Traits, Alloc>& format, Parsable& tp,
| ^~~~~
include/date/date.h:8123:1: note: candidate expects 4 arguments, 2 provided
include/date/date.h:8137:1: note: candidate: ‘template<class Parsable, class CharT> decltype ((date::from_stream(declval<std::basic_istream<CharT, std::char_traits<_CharT> >&>(), format, tp), date::parse_manip<Parsable, CharT>{format, tp})) date::parse(const CharT
, Parsable&)’
8137 | parse(const CharT* format, Parsable& tp)
| ^~~~~
include/date/date.h:8137:1: note: template argument deduction/substitution failed:
include/date/date.h: In substitution of ‘template<class Parsable, class CharT> decltype ((date::from_stream(declval<std::basic_istream<CharT, std::char_traits<_CharT> >&>(), format, tp), date::parse_manip<Parsable, CharT>{format, tp})) date::parse(const CharT*, Parsable&) [with Parsable = std::chrono::time_point<std::chrono::_V2::system_clock, std::chrono::duration<long int, std::ratio<1, 1000000000> > >; CharT = char]’:
src/reduct/client.cc:60:24: required from here
60 | date::parse("%FT%TZ", server_info.license->expiry_date);
| ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/date/date.h:8138:28: error: call of overloaded ‘from_stream(std::basic_istream&, const char*&, std::chrono::time_point<std::chrono::_V2::system_clock, std::chrono::duration<long int, std::ratio<1, 1000000000> > >&)’ is ambiguous
8138 | -> decltype(from_stream(std::declval<std::basic_istream&>(), format, tp),
| ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/date/date.h:8000:1: note: candidate: ‘std::basic_istream<_CharT, _Traits>& date::from_stream(std::basic_istream<_CharT, _Traits>&, const CharT*, sys_time&, std::__cxx11::basic_string<_CharT, _Traits, _Alloc>, std::chrono::minutes) [with Duration = std::chrono::duration<long int, std::ratio<1, 1000000000> >; CharT = char; Traits = std::char_traits; Alloc = std::allocator; sys_time = std::chrono::time_point<std::chrono::_V2::system_clock, std::chrono::duration<long int, std::ratio<1, 1000000000> > >; std::chrono::minutes = std::chrono::duration<long int, std::ratio<60> >]’
8000 | from_stream(std::basic_istream<CharT, Traits>& is, const CharT* fmt,
| ^~~~~~~~~~~
In file included from /usr/include/c++/14/chrono:3360,
from src/reduct/client.h:6,
from src/reduct/client.cc:3:
/usr/include/c++/14/bits/chrono_io.h:2733:5: note: candidate: ‘std::basic_istream<_CharT, _Traits>& std::chrono::from_stream(std::basic_istream<_CharT, _Traits>&, const _CharT*, sys_time<_Duration>&, std::__cxx11::basic_string<_CharT, _Traits, _Alloc>, minutes) [with _CharT = char; _Traits = std::char_traits; _Duration = duration<long int, std::ratio<1, 1000000000> >; _Alloc = std::allocator; sys_time<_Duration> = time_point<_V2::system_clock, duration<long int, std::ratio<1, 1000000000> > >; minutes = duration<long int, std::ratio<60> >]’
2733 | from_stream(basic_istream<_CharT, _Traits>& __is, const _CharT* __fmt,
| ^~~~~~~~~~~
include/date/date.h:8147:1: note: candidate: ‘template<class Parsable, class CharT, class Traits, class Alloc> decltype ((date::from_stream(declval<std::basic_istream<_CharT, _Traits>&>(), format, tp, (& abbrev)), date::parse_manip<Parsable, CharT, Traits, Alloc>{format, tp, (& abbrev)})) date::parse(const CharT*, Parsable&, std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&)’
8147 | parse(const CharT* format, Parsable& tp, std::basic_string<CharT, Traits, Alloc>& abbrev)
| ^~~~~
include/date/date.h:8147:1: note: candidate expects 3 arguments, 2 provided
include/date/date.h:8158:1: note: candidate: ‘template<class Parsable, class CharT> decltype ((date::from_stream(declval<std::basic_istream<CharT, std::char_traits<_CharT> >&>(), format, tp, declval<std::__cxx11::basic_string<_CharT, std::char_traits<_CharT>, std::allocator<_T2> >>(), (& offset)), date::parse_manip<Parsable, CharT>{format, tp, nullptr, (& offset)})) date::parse(const CharT, Parsable&, std::chrono::minutes&)’
8158 | parse(const CharT* format, Parsable& tp, std::chrono::minutes& offset)
| ^~~~~
include/date/date.h:8158:1: note: candidate expects 3 arguments, 2 provided
include/date/date.h:8169:1: note: candidate: ‘template<class Parsable, class CharT, class Traits, class Alloc> decltype ((date::from_stream(declval<std::basic_istream<_CharT, _Traits>&>(), format, tp, (& abbrev), (& offset)), date::parse_manip<Parsable, CharT, Traits, Alloc>{format, tp, (& abbrev), (& offset)})) date::parse(const CharT*, Parsable&, std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&, std::chrono::minutes&)’
8169 | parse(const CharT* format, Parsable& tp,
| ^~~~~

What was changed?

use std::chrono::parse instead of date::parse

Related issues

nothing.

Does this PR introduce a breaking change?

nothing.

Other information:

nothing.

@atimin
Copy link
Member

atimin commented May 11, 2024

Hey @toge , I'm not sure about this. The std::chrono::parse function was added only in GCC13, it is quite new. Looks like Clang still doesn't support this. Why do you want to get rid of the date package?

@toge
Copy link
Contributor Author

toge commented May 12, 2024

@atimin
As you can see from the error message, if std::chrono::parse and date::parse are implemented, the call to parse() is ambiguous and causes an error.

Considering clang, I need an implementation that changes the parse to be called depending on whether std::chrono::parse is implemented.

I'm going to find out the way to solve.

@toge
Copy link
Contributor Author

toge commented May 22, 2024

@atimin
At this time, gcc 14 appears to be the only compiler that supports std::chrono::parse.
https://godbolt.org/z/Y3zYx7PMr

I add REDUCT_CPP_USE_STD_CHRONO option to CMakeLists.txt.
If REDUCT_CPP_USE_STD_CHRONO is ON, reduct_cpp will use std::chrono::parse instead of date::parse.

Please review this or modify it for your product.
I am not concerned about this PR being merged.

@atimin
Copy link
Member

atimin commented May 24, 2024

Hey @toge, it looks good to me, I'll update the change log and merge it. Thank you for your contribution!

@atimin atimin merged commit d90c7bc into reductstore:main May 24, 2024
7 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants