replace boost lexical cast with stl string #195
Conversation
Signed-off-by: Michael T. Wells <[email protected]>
wellsmt
changed the title
|
per @JeffGarland recommendation, std::stoul() is a c++11 feature and will need to use |
| @@ -15,7 +15,6 @@ | |||
| #include <iterator> | |||
| #include <algorithm> | |||
| #include <boost/tokenizer.hpp> | |||
| #include <boost/lexical_cast.hpp> | |||
There was a problem hiding this comment.
So here we should ifdef the include so that if we're supporting pre-11 c++ we will just keep using lexical cast probably the cleanest solution is to move it to the compiler_config.hpp to do the macro check. I looked and can't specific feature macro for these functions so I think I'd go with
BOOST_NO_CXX11
boost.config is already included so you should have direct access to this.
There was a problem hiding this comment.
Excellent!
...clarifying question...
Is the goal to totally eliminate the dependency on boost/lexical_cast? If so then we could use cstdlib for < c++11 and string for >= c++11.
Or is the goal simply to modernize the code? If so we can use the macro approach you mentioned for < c++11 and leave the dependency on boost::lexical_cast.
There was a problem hiding this comment.
I'm fine sticking with lexical_cast for 03 compiles and just modernizing for 11 and beyond. It seems likely I'll eventually drop 03 support.
| @@ -15,7 +15,6 @@ | |||
| #include <iterator> | |||
| #include <algorithm> | |||
| #include <boost/tokenizer.hpp> | |||
There was a problem hiding this comment.
I forgot about this one as well -- probably this will be next on my radar.
| @@ -61,7 +60,7 @@ namespace date_time { | |||
| inline unsigned short | |||
| month_str_to_ushort(std::string const& s) { | |||
| if((s.at(0) >= '0') && (s.at(0) <= '9')) { | |||
| return boost::lexical_cast<unsigned short>(s); | |||
There was a problem hiding this comment.
so my suggestion is we write the function
#ifdef BOOST_NO_CXX11
inline unsigned short string_to_ushort(...)
{
--> current lexical cast
}
#else
inline unsigned short string_to_ushort(...)
{
--> current stoul code
}
#endif
This can live here or if it's used elsewhere I'd move it to compiler_config.hpp since its a work around
|
I was able to kick the ci |
|
And sorta obviously the at least the 03 compiles aren't happy with those functions. |
- string when < c++11 - lexical_cast when >=c++11 Signed-off-by: Michael T. Wells <[email protected]>
|
Looking good -- let's see what the CI sez. |
- TODO: lexical cast called because of unknown needed side effect for gregorian tests Signed-off-by: Michael T. Wells <[email protected]>
| return boost::lexical_cast<unsigned short>(s); | ||
| #else | ||
| // TODO: lexical cast has some side effect on <s> that makes the std::stoul work here | ||
| boost::lexical_cast<unsigned short>(s); |
There was a problem hiding this comment.
@JeffGarland This line should be removed, but i was curious how it would run in CI. In my local environment, the tests pass when the lexical_cast is called on s, but fail when it is not called. There is some side effect of boost::lexical_cast on s that I don't understand in the current date_time uninttests. In particular this test i have been looking at here fails without doing this cast.
|
@wellsmt rebase on develop for a complete CI scan |
The purpose of this request to replace the dependency on boost::lexical cast with std string conversions for
c++ >= c++11and leaveboost::lexical_castfor< c++11.Signed-off-by: Michael T. Wells [email protected]