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

Explicit cast #636

Merged
merged 7 commits into from Mar 23, 2020
Merged

Explicit cast #636

merged 7 commits into from Mar 23, 2020

Conversation

@Thrameos
Copy link
Contributor

Thrameos commented Mar 21, 2020

Tonight's PR deals with explicit casting to primitives. In Java casting will never produce an overflow error, thus we have replicated that behavior. Implicit conversions still produce OverflowError as per Python conventions. This was intended to be a very small pull request but unfortunately the casting operations required a different field for each type for proper sign convention which ran straight into the big bag of if statements in PyJPValue_create. Those if statements were replaced with logic in the convertToPythonObject methods which were already part of the system. A secondary bug with overflow from unsigned long was also deal with, but there may be an additional one with array assignments.

Fixes #602

@Thrameos Thrameos requested a review from marscher Mar 21, 2020
Copy link
Contributor Author

Thrameos left a comment

Highlights of the changes.

virtual JPPyObject convertToPythonObject(JPJavaFrame& frame, jvalue val);
virtual JPPyObject convertToPythonObject(JPJavaFrame& frame, jvalue val, bool cast);
Comment on lines -137 to +143

This comment has been minimized.

Copy link
@Thrameos

Thrameos Mar 21, 2020

Author Contributor

We are adding an extra "cast" argument. If set to true it disables all of the Python conversions and forces the type to match.

JPClass *cls = this;
if (!cast)
{
// This loses type
if (value.l == NULL)
{
return JPPyObject::getNone();
}

cls = frame.findClassForObject(value.l);
if (cls != this)
return cls->convertToPythonObject(frame, value, true);
}
Comment on lines +65 to +77

This comment has been minimized.

Copy link
@Thrameos

Thrameos Mar 21, 2020

Author Contributor

The non casting section covers conversion to None for null pointers and looking up the best wrapper type.

// Disable OverrangeError
match.type = JPMatch::_exact;
val = match.convert();
converted = true;
PyObject *obj = cls->convertToPythonObject(frame, val, true).keep();
return obj;
Comment on lines +53 to +57

This comment has been minimized.

Copy link
@Thrameos

Thrameos Mar 21, 2020

Author Contributor

When creating a primitive as a cast we will set the type of the match to exact after we find it to force the conversion to bypass overflow error.

if (val == -1)
JP_PY_CHECK();
base_t::field(res) = (typename base_t::type_t) base_t::assertRange(val);
}

This comment has been minimized.

Copy link
@Thrameos

Thrameos Mar 21, 2020

Author Contributor

We select the type of conversion here. We need different functions based on the whether it is an implicit or a user requested cast.

PyObject *JPPrimitiveType::convertLong(PyTypeObject* wrapper, PyLongObject* tmp)
{
if (wrapper == NULL)
JP_RAISE(PyExc_SystemError, "bad wrapper");
Py_ssize_t n = Py_SIZE(tmp);
if (n < 0)
n = -n;

PyLongObject *newobj = (PyLongObject *) wrapper->tp_alloc(wrapper, n);
if (newobj == NULL)
return NULL;

((PyVarObject*) newobj)->ob_size = Py_SIZE(tmp);
for (Py_ssize_t i = 0; i < n; i++)
{
newobj->ob_digit[i] = tmp->ob_digit[i];
}
return (PyObject*) newobj;
}
Comment on lines +36 to +54

This comment has been minimized.

Copy link
@Thrameos

Thrameos Mar 21, 2020

Author Contributor

Unfortunately the Python API does not have an easy way to call long_subtype_new from outside of Python so we have to replicate the functionality here.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Mar 22, 2020

Codecov Report

Merging #636 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #636   +/-   ##
=======================================
  Coverage   89.73%   89.73%           
=======================================
  Files          24       24           
  Lines        1529     1529           
=======================================
  Hits         1372     1372           
  Misses        157      157
Impacted Files Coverage Δ
jpype/_core.py 91.32% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c3118f9...15ca358. Read the comment docs.

@Thrameos Thrameos merged commit d63a6e5 into jpype-project:master Mar 23, 2020
5 checks passed
5 checks passed
LGTM analysis: Java No code changes detected
Details
LGTM analysis: C/C++ No new or fixed alerts
Details
LGTM analysis: Python No new or fixed alerts
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Thrameos Thrameos deleted the Thrameos:explicit_cast branch Mar 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.