Skip to content

Throw std::runtime_exception instead of char*. - #9289

Merged
tacaswell merged 1 commit into
matplotlib:masterfrom
anntzer:dont-throw-cpp-strings
Oct 6, 2017
Merged

tacaswell merged 1 commit into
matplotlib:masterfrom
anntzer:dont-throw-cpp-strings

Conversation

@anntzer

@anntzer anntzer commented Oct 5, 2017

Copy link
Copy Markdown
Contributor

This should make it easier to debug exceptions falling out of the C++
code, as the C++ runtime will print the error message (the what() of
the exception) on its way to aborting the program.

Did a regex replace; removed the char* handler in CALL_CPP_FULL; and
remove a bunch of extra whitespace.

For example, before application of the patch in #9074, the failing code now gives

terminate called after throwing an instance of 'std::runtime_error'
  what():  Couldn't close file
Fatal Python error: Aborted

Should also help with #9244.

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

This should make it easier to debug exceptions falling out of the C++
code, as the C++ runtime will print the error message (the `what()` of
the exception) on its way to aborting the program.

Did a regex replace; removed the `char*` handler in CALL_CPP_FULL; and
remove a bunch of extra whitespace.
@tacaswell

Copy link
Copy Markdown
Member

Would prefer @mdboom or @ianthomas23 sign off on this before merging.

@ianthomas23 ianthomas23 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with this.

Don't understand the AppVeyor failure.

@tacaswell tacaswell added this to the 2.1.1 (next bug fix release) milestone Oct 6, 2017
@tacaswell

Copy link
Copy Markdown
Member

We don't either yet, see #9176 and #9282

@tacaswell
tacaswell merged commit 89183ba into matplotlib:master Oct 6, 2017
@anntzer
anntzer deleted the dont-throw-cpp-strings branch October 6, 2017 20:34
@Kojoley

Kojoley commented Oct 6, 2017

Copy link
Copy Markdown
Member

Hm. CALL_CPP_FULL is used to catch C++ exceptions and convert them to Python. std::runtime_error is not handled by it (you just simply removed char const *e), so all these exception now will be caught by catch (...) and raise in Python with "Unknown exception in %s" message.
Am I wrong?

@anntzer

anntzer commented Oct 6, 2017

Copy link
Copy Markdown
Contributor Author

Oops. New PR coming.

QuLogic added a commit that referenced this pull request Oct 6, 2017
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.

4 participants