Skip to content

fix compilation errors with exceptions disabled #1355

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

Merged
merged 2 commits into from
Jan 31, 2025

Conversation

jmplanes02
Copy link
Contributor

@jmplanes02 jmplanes02 commented Jan 14, 2025

Currently, compiling with -fno-exceptions will not work because cpp2util.h uses try-catch and throw without checking if the macro CPP2_NO_EXCEPTIONS is defined.

I've done the following and now compiling with exceptions disabled works fine:

  • Give cpp2::string_util::string_to_int function an implementation when CPP2_NO_EXCEPTIONS is set
  • Use Throw instead of throw inside cpp2::range constructor

@hsutter
Copy link
Owner

hsutter commented Jan 15, 2025

Thanks! It looks like this may be your first contribution to cppfront. I don't have your email, so please send me email with your full name, and I'll send the Contributor License Agreement (CLA), and once it's signed I can look in more detail at your pull request. Thanks again for your contribution.

@jmplanes02
Copy link
Contributor Author

I've just made my email public on my GitHub account, but in any case, I’ve also sent you my information to your gmail.

And add an assertion for `end` being not null, which `strtol` guarantees but it's good to check it anyway - and that needs moving contracts earlier in the file
@hsutter
Copy link
Owner

hsutter commented Jan 31, 2025

Looks good, thanks!

@hsutter hsutter merged commit 8697e33 into hsutter:main Jan 31, 2025
@hsutter
Copy link
Owner

hsutter commented Jan 31, 2025

Oh, by the way I inadvertently left in a temporary change from #ifndef CPP2_NO_EXCEPTIONS to #ifdef CPP2_NO_EXCEPTIONS (as an expedient way to test the other path). I'll change that back as a separate commit.

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