Commit Graph

11 Commits

Author SHA1 Message Date
luz.paz
ed0a72eb0e misc. typos
Found via `codespell`
2018-02-07 10:54:31 +01:00
Jason Rhinelander
1682b67326 Simplify error_already_set
`error_already_set` is more complicated than it needs to be, partly
because it manages reference counts itself rather than using
`py::object`, and partly because it tries to do more exception clearing
than is needed.  This commit greatly simplifies it, and fixes #927.

Using `py::object` instead of `PyObject *` means we can rely on
implicit copy/move constructors.

The current logic did both a `PyErr_Clear` on deletion *and* a
`PyErr_Fetch` on creation.  I can't see how the `PyErr_Clear` on
deletion is ever useful: the `Fetch` on creation itself clears the
error, so the only way doing a `PyErr_Clear` on deletion could do
anything if is some *other* exception was raised while the
`error_already_set` object was alive--but in that case, clearing some
other exception seems wrong.  (Code that is worried about an exception
handler raising another exception would already catch a second
`error_already_set` from exception code).

The destructor itself called `clear()`, but `clear()` was a little bit
more paranoid that needed: it called `restore()` to restore the
currently captured error, but then immediately cleared it, using the
`PyErr_Restore` to release the references.  That's unnecessary: it's
valid for us to release the references manually.  This updates the code
to simply release the references on the three objects (preserving the
gil acquire).

`clear()`, however, also had the side effect of clearing the current
error, even if the current `error_already_set` didn't have a current
error (e.g. because of a previous `restore()` or `clear()` call).  I
don't really see how clearing the error here can ever actually be
useful: the only way the current error could be set is if you called
`restore()` (in which case the current stored error-related members have
already been released), or if some *other* code raised the error, in
which case `clear()` on *this* object is clearing an error for which it
shouldn't be responsible.

Neither of those seem like intentional or desirable features, and
manually requesting deletion of the stored references similarly seems
pointless, so I've just made `clear()` an empty method and marked it
deprecated.

This also fixes a minor potential issue with the destruction: it is
technically possible for `value` to be null (though this seems likely to
be rare in practice); this updates the check to look at `type` which
will always be non-null for a `Fetch`ed exception.

This also adds error_already_set round-trip throw tests to the test
suite.
2017-07-28 20:40:35 -04:00
Jason Rhinelander
abcf43d59c Convert test_exceptions to new testing style 2017-07-28 20:40:35 -04:00
Dean Moldovan
83e328f58c Split test_python_types.cpp into builtin_casters, stl and pytypes 2017-06-27 10:38:41 +02:00
Roman Miroshnychenko
83a8a977a7 Add a method to check Python exception types (#772)
This commit adds `error_already_set::matches()` convenience method to
check if the exception trapped by `error_already_set` matches a given
Python exception type. This will address #700 by providing a less
verbose way to check exceptions.
2017-04-02 22:38:50 +02:00
Jason Rhinelander
3f1ff3f4d1 Adds automatic casting on assignment of non-pyobject types (#551)
This adds automatic casting when assigning to python types like dict,
list, and attributes.  Instead of:

    dict["key"] = py::cast(val);
    m.attr("foo") = py::cast(true);
    list.append(py::cast(42));

you can now simply write:

    dict["key"] = val;
    m.attr("foo") = true;
    list.append(42);

Casts needing extra parameters (e.g. for a non-default rvp) still
require the py::cast() call. set::add() is also supported.

All usage is channeled through a SFINAE implementation which either just returns or casts. 

Combined non-converting handle and autocasting template methods via a
helper method that either just returns (handle) or casts (C++ type).
2016-12-12 23:42:52 +01:00
Jason Rhinelander
b3794f1087 Added py::register_exception for simple case (#296)
The custom exception handling added in PR #273 is robust, but is overly
complex for declaring the most common simple C++ -> Python exception
mapping that needs only to copy `what()`.  This add a simpler
`py::register_exception<CppExp>(module, "PyExp");` function that greatly
simplifies the common basic case of translation of a simple CppException
into a simple PythonException, while not removing the more advanced
capabilities of defining custom exception handlers.
2016-09-16 08:04:15 +02:00
Dean Moldovan
135ba8deaf Make error_already_set fetch and hold the Python error
This clears the Python error at the error_already_set throw site, thus
allowing Python calls to be made in destructors which are triggered by
the exception. This is preferable to the alternative, which would be
guarding every Python API call with an error_scope.

This effectively flips the behavior of error_already_set. Previously,
it was assumed that the error stays in Python, so handling the exception
in C++ would require explicitly calling PyErr_Clear(), but nothing was
needed to propagate the error to Python. With this change, handling the
error in C++ does not require a PyErr_Clear() call, but propagating the
error to Python requires an explicit error_already_set::restore().

The change does not break old code which explicitly calls PyErr_Clear()
for cleanup, which should be the majority of user code. The need for an
explicit restore() call does break old code, but this should be mostly
confined to the library and not user code.
2016-09-10 12:08:32 +02:00
Ivan Smirnov
67b54894b2 Set error if it's not set in error_already_set() 2016-09-07 21:10:16 +01:00
Jason Rhinelander
52f4be8946 Make test initialization self-registering
Adding or removing tests is a little bit cumbersome currently: the test
needs to be added to CMakeLists.txt, the init function needs to be
predeclared in pybind11_tests.cpp, then called in the plugin
initialization.  While this isn't a big deal for tests that are being
committed, it's more of a hassle when working on some new feature or
test code for which I temporarily only care about building and linking
the test being worked on rather than the entire test suite.

This commit changes tests to self-register their initialization by
having each test initialize a local object (which stores the
initialization function in a static variable).  This makes changing the
set of tests being build easy: one only needs to add or comment out
test names in tests/CMakeLists.txt.

A couple other minor changes that go along with this:

- test_eigen.cpp is now included in the test list, then removed if eigen
  isn't available.  This lets you disable the eigen tests by commenting
  it out, just like all the other tests, but keeps the build working
  without eigen eigen isn't available.  (Also, if it's commented out, we
  don't even bother looking for and reporting the building with/without
  eigen status message).

- pytest is now invoked with all the built test names (with .cpp changed
  to .py) so that it doesn't try to run tests that weren't built.
2016-09-03 17:34:41 -04:00
Dean Moldovan
a0c1ccf0a9 Port tests to pytest
Use simple asserts and pytest's powerful introspection to make testing
simpler. This merges the old .py/.ref file pairs into simple .py files
where the expected values are right next to the code being tested.

This commit does not touch the C++ part of the code and replicates the
Python tests exactly like the old .ref-file-based approach.
2016-08-19 13:19:38 +02:00