Exceptionless lexical_cast

[Olaf van der Spek <olafvdspek@…>]

Could you provide an exceptionless lexical_cast variant?

bool try_lexical_cast<Out, In>(Out&, const In&)

Exceptions aren't always the best option. Sometimes they're not even an option. Could you provide an exceptionless variant? As bonus it's no longer necessary to specify the Out template argument.

[Antony Polukhin]

Currently it is not possible, because I can not provide strong exception guarantee. For example, lexical_cast uses std::locale, which can throw. String constructors, some std::stream operators, user defined ostream and istream operators may also throw.

[Olaf van der Spek <olafvdspek@…>]

I'm not asking for a variant that doesn't throw at all, just asking for a variant that doesn't throw itself. If some other code throws, that's ok.

[Antony Polukhin]

Replying to Olaf van der Spek <olafvdspek@…>:

I'm not asking for a variant that doesn't throw at all, just asking for a variant that doesn't throw itself. If some other code throws, that's ok.

I like the idea of try_lexical_cast, but only if it provides strong exception guarantee. Otherwise it may obfuscate users and is not as useful as function with strong exception guarantee.

[Olaf van der Spek <olafvdspek@…>]

I assume that's doable.

[ben.maurer@…]

Any update on if you'd consider doing this? At Facebook we've seen many instances of programs that call lexical_cast in a try/catch. In cases where the parsing is expected to fail (eg, an argument could be an int or a date) we frequently see extremely poor performance in multithreading. GCC is known to have poor multithreaded exception handling gcc.gnu.org/ml/gcc/2013-05/msg00253.html

Facebook is currently running a bug bounty program for open source bugs, and this is a bug that might be worthy of such a bounty, if is something there is interest in merging.

[Antony Polukhin]

This issue hit me in my own projects.

I'll create a separate branch at github with try_lexical_cast and ask for mini review at mailing lists when it'll be ready.

[ben.maurer@…]

Thanks! Facebook is putting a $200 bounty on this via bountysource: www.bountysource.com/issues/1352296

[Troy Korjuslommi <troykor@…>]

Patch to add try_lexical_cast function. Patch is against boost_1_55_0 lexical_cast.hpp.

[Troy Korjuslommi <troykor@…>]

I added the patch here.

try_lexical_cast: A version which doesn't throw bad_lexical_cast. All other exceptions will be thrown as normal.

Getting rid of exceptions thrown in the callbacks nothrow_overflow_handler and detect_precision_loss would mean rewriting boost::numeric::converter, which seems excessive, and futile. And it would create a dependency on those patches going through for this patch to be accepted. Ergo, I resorted to catching these exceptions to provide non throwing behavior.

All other instances where an instance of bad_lexical_cast could be thrown have been replaced with a new function.

Dropped the deprecated call-by-value fallback version template<typename Target, typename Source> bool bool try_lexical_cast(Target& result, Source arg);

[Antony Polukhin]

Replying to Troy Korjuslommi <troykor@…>:

I added the patch here.

That's a bad patch. It breaks compilation with exceptions off. It does not add tests for new feature and does not update the docs.

It is also much worse than an already applied ​to develop branch patch:

try {} catch(...){} is not what was requested by users. boost::numeric::converter was the main problem! Without fixing it a patch could be written in 10 minutes, but that would be a patch for outward appearances, does not really resolving the issue.

[Troy Korjuslommi <troykor@…>]

I can post proper tests and docs once this patch gets some positive comments. The comments in this ticket seem sufficient for reviewing the patch itself.

The lexical_cast function doesn't compile with -DBOOST_NO_EXCEPTIONS either, and this ticket had no mention of it. I did consider it, but it didn't seem to be an concern here. I guess I was wrong. I will see what needs to be done.

The patch against numeric::converter should be developed independently, as it's a separate package. Once it works, try_lexical_cast can be modified to call it's non throwing function. That's a one line change.

As I see it, the patch solves the issue at hand. Adding the -DBOOST_NO_EXCEPTIONS support and calls to modified numeric::converter can be added once the basic functionality is approved.

[Antony Polukhin]

Replying to Troy Korjuslommi <troykor@…>:

I can post proper tests and docs once this patch gets some positive comments. The comments in this ticket seem sufficient for reviewing the patch itself.

Once again: the issue is already resolved in developer branch of Boost.

The lexical_cast function doesn't compile with -DBOOST_NO_EXCEPTIONS either, and this ticket had no mention of it. I did consider it, but it didn't seem to be an concern here. I guess I was wrong. I will see what needs to be done.

You are wrong. lexical_cast function does compile with -DBOOST_NO_EXCEPTIONS. See results for lexical_cast_no_exceptions_test. And it compiles for a long time!

The patch against numeric::converter should be developed independently, as it's a separate package. Once it works, try_lexical_cast can be modified to call it's non throwing function. That's a one line change.

As I see it, the patch solves the issue at hand. Adding the -DBOOST_NO_EXCEPTIONS support and calls to modified numeric::converter can be added once the basic functionality is approved.

Once again: the issue is already resolved in developer branch of Boost. It is waiting for regression tests to pass, before merging to release. And it is resolved without breaking existing functionality and without "let's do it wrong for now".

If you want to help, try fixing this issue #8261.

If you want to get some easy money from ​bountysource, then do the work right! Do the work that is not finished!

[Troy Korjuslommi <troykor@…>]

Next time you work on something, please mention it here too, and provide a link for others to see your work. Now I wasted my time.

[Antony Polukhin]

Replying to Troy Korjuslommi <troykor@…>:

Next time you work on something, please mention it here too, and provide a link for others to see your work. Now I wasted my time.

Mention at ​comment 6 does not work for you?

It's a shame that "getting new knowledge" for you is "wasting time".

[Troy Korjuslommi <troykor@…>]

I will have some time later today, so I will have a look at the #8261 issue.

I did look for try_lexical_cast under github, but it wasn't there. I assumed you had abandoned the task. I realize now I should have used google and not github's own search function to look for the project, since you had apparently changed your mind and posted it under conversion instead of creating a try_lexical_cast entry.

Because I didn't see the entry, I used the latest version of boost, 1.55.0 as the basis for the changes. This seemed sensible at the time. 1.55.0's lexical_cast does not compile with -DBOOST_NO_EXCEPTIONS, but the development branch does. There's the cause for this miscommunication.

Anyway, no need to be upset. I wasn't trying to step on your toes, just thought I'd contribute something. You have the changes under control. That's great. I will have a look at the problem with #8261 and let's see if I can help there.

[Antony Polukhin]

​Merged to master branch, after ensuring that test failures are not related to try_lexical_convert changes.