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

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

Merged
merged 1 commit into from Oct 6, 2017

Conversation

Projects
None yet
4 participants
@anntzer
Copy link
Contributor

commented Oct 5, 2017

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
Throw std::runtime_exception instead of char*.
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

This comment has been minimized.

Copy link
Member

commented Oct 6, 2017

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

@ianthomas23 ianthomas23 added color/alpha and removed color/alpha labels Oct 6, 2017

@ianthomas23
Copy link
Member

left a comment

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

This comment has been minimized.

Copy link
Member

commented Oct 6, 2017

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

@tacaswell tacaswell merged commit 89183ba into matplotlib:master Oct 6, 2017

7 of 8 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
ci/circleci: docs-python27 Your tests passed on CircleCI!
Details
ci/circleci: docs-python35 Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing 589b953...df79954
Details
codecov/project/library 61.66% (target 50%)
Details
codecov/project/tests 98.72% remains the same compared to 589b953
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
lgtm analysis: Python No alert changes
Details

meeseeksdev bot pushed a commit that referenced this pull request Oct 6, 2017

@anntzer anntzer deleted the anntzer:dont-throw-cpp-strings branch Oct 6, 2017

@Kojoley

This comment has been minimized.

Copy link
Member

commented Oct 6, 2017

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

This comment has been minimized.

Copy link
Contributor Author

commented Oct 6, 2017

Oops. New PR coming.

@anntzer anntzer referenced this pull request Oct 6, 2017

Merged

Restore better error message on std::runtime_error. #9299

0 of 6 tasks complete

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
You can’t perform that action at this time.