* [PATCH] Implement floordiv operator for gdb.Value
@ 2016-09-20 13:34 Jonathan Wakely
2016-09-20 14:46 ` Jonathan Wakely
2016-09-20 15:41 ` Pedro Alves
0 siblings, 2 replies; 10+ messages in thread
From: Jonathan Wakely @ 2016-09-20 13:34 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 961 bytes --]
This is my attempt to implement the // operator on gdb.Value objects.
There is already BINOP_INTDIV which works fine for integral types, but
for floats I use BINOP_DIV and then call floor() on the result. This
doesn't support decimal floats though.
Is this a reasonable solution? Is the test sufficient?
I have a follow-up patch which changes the meaning of the / operator
for gdb.Value when built against Python 3, to be consistent with
Python (see comment 1 in the Bugzilla PR) but I expect that to be more
controversial :-)
gdb/ChangeLog:
2016-09-20 Jonathan Wakely <jwakely@redhat.com>
PR python/20624
* python/py-value.c (VALPY_FLOORDIV): Define new enumerator.
(valpy_binop_throw): Handle VALPY_FLOORDIV.
(valpy_floordiv): New function.
(value_object_as_number): Set valpy_floordiv in relevant slot.
gdb/testsuite/ChangeLog:
2016-09-20 Jonathan Wakely <jwakely@redhat.com>
PR python/20624
* gdb.python/py-value.exp: Test floor division.
[-- Attachment #2: floordiv.txt --]
[-- Type: text/plain, Size: 4343 bytes --]
commit 3a019f3ba61bde5320419f5782d8f2554ac55ace
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Tue Sep 20 10:38:35 2016 +0100
gdb: Implement floordiv operator for gdb.Value
gdb/ChangeLog:
2016-09-20 Jonathan Wakely <jwakely@redhat.com>
PR python/20624
* python/py-value.c (VALPY_FLOORDIV): Define new enumerator.
(valpy_binop_throw): Handle VALPY_FLOORDIV.
(valpy_floordiv): New function.
(value_object_as_number): Set valpy_floordiv in relevant slot.
gdb/testsuite/ChangeLog:
2016-09-20 Jonathan Wakely <jwakely@redhat.com>
PR python/20624
* gdb.python/py-value.exp: Test floor division.
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 21e9247..f6a6c11 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -1011,7 +1011,8 @@ enum valpy_opcode
VALPY_RSH,
VALPY_BITAND,
VALPY_BITOR,
- VALPY_BITXOR
+ VALPY_BITXOR,
+ VALPY_FLOORDIV
};
/* If TYPE is a reference, return the target; otherwise return TYPE. */
@@ -1032,6 +1033,7 @@ valpy_binop_throw (enum valpy_opcode opcode, PyObject *self, PyObject *other)
struct value *res_val = NULL;
enum exp_opcode op = OP_NULL;
int handled = 0;
+ bool floor_it = false;
/* If the gdb.Value object is the second operand, then it will be
passed to us as the OTHER argument, and SELF will be an entirely
@@ -1113,6 +1115,22 @@ valpy_binop_throw (enum valpy_opcode opcode, PyObject *self, PyObject *other)
case VALPY_REM:
op = BINOP_REM;
break;
+ case VALPY_FLOORDIV:
+ {
+ struct type *ltype = value_type (arg1);
+ struct type *rtype = value_type (arg2);
+ ltype = check_typedef (ltype);
+ rtype = check_typedef (rtype);
+ if (TYPE_CODE (ltype) == TYPE_CODE_FLT
+ || TYPE_CODE (rtype) == TYPE_CODE_FLT)
+ {
+ op = BINOP_DIV;
+ floor_it = true;
+ }
+ else
+ op = BINOP_INTDIV;
+ }
+ break;
case VALPY_POW:
op = BINOP_EXP;
break;
@@ -1142,7 +1160,15 @@ valpy_binop_throw (enum valpy_opcode opcode, PyObject *self, PyObject *other)
}
if (res_val)
- result = value_to_value_object (res_val);
+ {
+ if (floor_it)
+ {
+ double d = value_as_double (res_val);
+ d = floor (d);
+ res_val = value_from_double (value_type (res_val), d);
+ }
+ result = value_to_value_object (res_val);
+ }
do_cleanups (cleanup);
return result;
@@ -1200,6 +1226,12 @@ valpy_remainder (PyObject *self, PyObject *other)
}
static PyObject *
+valpy_floordiv (PyObject *self, PyObject *other)
+{
+ return valpy_binop (VALPY_FLOORDIV, self, other);
+}
+
+static PyObject *
valpy_power (PyObject *self, PyObject *other, PyObject *unused)
{
/* We don't support the ternary form of pow. I don't know how to express
@@ -1837,7 +1869,7 @@ static PyNumberMethods value_object_as_number = {
NULL, /* nb_inplace_and */
NULL, /* nb_inplace_xor */
NULL, /* nb_inplace_or */
- NULL, /* nb_floor_divide */
+ valpy_floordiv, /* nb_floor_divide */
valpy_divide, /* nb_true_divide */
NULL, /* nb_inplace_floor_divide */
NULL, /* nb_inplace_true_divide */
diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp
index 57a9ba1..81837e9 100644
--- a/gdb/testsuite/gdb.python/py-value.exp
+++ b/gdb/testsuite/gdb.python/py-value.exp
@@ -87,6 +87,8 @@ proc test_value_numeric_ops {} {
gdb_test "python print ('result = ' + str(f/g))" " = 0.5" "divide two double values"
gdb_test "python print ('result = ' + str(i%j))" " = 1" "take remainder of two integer values"
# Remainder of float is implemented in Python but not in GDB's value system.
+ gdb_test "python print ('result = ' + str(i//j))" " = 2" "floor-divide two integer values"
+ gdb_test "python print ('result = ' + str(f//g))" " = 0" "floor-divide two double values"
gdb_test "python print ('result = ' + str(i**j))" " = 25" "integer value raised to the power of another integer value"
gdb_test "python print ('result = ' + str(g**j))" " = 6.25" "double value raised to the power of integer value"
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Implement floordiv operator for gdb.Value
2016-09-20 13:34 [PATCH] Implement floordiv operator for gdb.Value Jonathan Wakely
@ 2016-09-20 14:46 ` Jonathan Wakely
2016-09-20 15:41 ` Pedro Alves
1 sibling, 0 replies; 10+ messages in thread
From: Jonathan Wakely @ 2016-09-20 14:46 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1441 bytes --]
On 20/09/16 14:26 +0100, Jonathan Wakely wrote:
>This is my attempt to implement the // operator on gdb.Value objects.
>There is already BINOP_INTDIV which works fine for integral types, but
>for floats I use BINOP_DIV and then call floor() on the result. This
>doesn't support decimal floats though.
>
>Is this a reasonable solution? Is the test sufficient?
>
>I have a follow-up patch which changes the meaning of the / operator
>for gdb.Value when built against Python 3, to be consistent with
>Python (see comment 1 in the Bugzilla PR) but I expect that to be more
>controversial :-)
Here's the follow-up, which I'm only posting for curiosity value.
It's a bit of a hack, possibly buggy, and needs more thought about
whether this is even a good idea.
From: Jonathan Wakely <jwakely@redhat.com>
Date: Tue, 20 Sep 2016 14:32:50 +0100
Subject: [PATCH] gdb: Make gdb.Value division consistent with Python
In Python 3.x dividing integers does not perform integer division, so
make gdb.Value behave the same way when built with Python 3.x or later.
gdb/ChangeLog:
2016-09-20 Jonathan Wakely <jwakely@redhat.com>
PR python/20624
* python/py-value.c (valpy_binop_throw) [PY_MAJOR_VERSION >= 3]:
Convert integers to double for VALPY_DIV to perform exact division.
gdb/testsuite/ChangeLog:
2016-09-20 Jonathan Wakely <jwakely@redhat.com>
PR python/20624
* gdb.python/py-value.exp: Adjust expected value for dividing two
integers.
[-- Attachment #2: truediv.txt --]
[-- Type: text/plain, Size: 2625 bytes --]
commit 03ded236edda388bb55d5c8678f1b754cab8ba8e
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Tue Sep 20 14:32:50 2016 +0100
gdb: Make gdb.Value division consistent with Python
In Python 3.x dividing integers does not perform integer division, so
make gdb.Value behave the same way when built with Python 3.x or later.
gdb/ChangeLog:
2016-09-20 Jonathan Wakely <jwakely@redhat.com>
PR python/20624
* python/py-value.c (valpy_binop_throw) [PY_MAJOR_VERSION >= 3]:
Convert integers to double for VALPY_DIV to perform exact division.
gdb/testsuite/ChangeLog:
2016-09-20 Jonathan Wakely <jwakely@redhat.com>
PR python/20624
* gdb.python/py-value.exp: Adjust expected value for dividing two
integers.
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index f6a6c11..28877be 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -1110,7 +1110,22 @@ valpy_binop_throw (enum valpy_opcode opcode, PyObject *self, PyObject *other)
op = BINOP_MUL;
break;
case VALPY_DIV:
+#if PY_MAJOR_VERSION < 3
op = BINOP_DIV;
+#else
+ {
+ struct type *ltype = value_type (arg1);
+ struct type *rtype = value_type (arg2);
+ ltype = check_typedef (ltype);
+ rtype = check_typedef (rtype);
+ if (is_integral_type (ltype) && is_integral_type (rtype))
+ {
+ long l = value_as_long (arg1);
+ arg1 = value_from_double (builtin_type_pyfloat, (double)l);
+ }
+ op = BINOP_DIV;
+ }
+#endif
break;
case VALPY_REM:
op = BINOP_REM;
diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp
index 81837e9..c1bdc06 100644
--- a/gdb/testsuite/gdb.python/py-value.exp
+++ b/gdb/testsuite/gdb.python/py-value.exp
@@ -83,7 +83,7 @@ proc test_value_numeric_ops {} {
gdb_test "python print ('result = ' + str(f-g))" " = -1.25" "subtract two double values"
gdb_test "python print ('result = ' + str(i*j))" " = 10" "multiply two integer values"
gdb_test "python print ('result = ' + str(f*g))" " = 3.125" "multiply two double values"
- gdb_test "python print ('result = ' + str(i/j))" " = 2" "divide two integer values"
+ gdb_test "python print ('result = ' + str(i/j))" " = 2.5" "divide two integer values"
gdb_test "python print ('result = ' + str(f/g))" " = 0.5" "divide two double values"
gdb_test "python print ('result = ' + str(i%j))" " = 1" "take remainder of two integer values"
# Remainder of float is implemented in Python but not in GDB's value system.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Implement floordiv operator for gdb.Value
2016-09-20 13:34 [PATCH] Implement floordiv operator for gdb.Value Jonathan Wakely
2016-09-20 14:46 ` Jonathan Wakely
@ 2016-09-20 15:41 ` Pedro Alves
2016-09-20 16:41 ` Jonathan Wakely
1 sibling, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2016-09-20 15:41 UTC (permalink / raw)
To: Jonathan Wakely, gdb-patches
Hi there,
Thanks!
On 09/20/2016 02:26 PM, Jonathan Wakely wrote:
> This is my attempt to implement the // operator on gdb.Value objects.
> There is already BINOP_INTDIV which works fine for integral types, but
> for floats I use BINOP_DIV and then call floor() on the result. This
> doesn't support decimal floats though.
>
> Is this a reasonable solution? Is the test sufficient?
>
See below.
> @@ -1142,7 +1160,15 @@ valpy_binop_throw (enum valpy_opcode opcode, PyObject *self, PyObject *other)
> }
>
> if (res_val)
> - result = value_to_value_object (res_val);
> + {
> + if (floor_it)
> + {
> + double d = value_as_double (res_val);
Should be s/double/DOUBLEST, I suppose?
> + d = floor (d);
> + res_val = value_from_double (value_type (res_val), d);
> + }
> + result = value_to_value_object (res_val);
> + }
>
> do_cleanups (cleanup);
> return result;
> @@ -1200,6 +1226,12 @@ valpy_remainder (PyObject *self, PyObject *other)
> }
>
> static PyObject *
> +valpy_floordiv (PyObject *self, PyObject *other)
> +{
> + return valpy_binop (VALPY_FLOORDIV, self, other);
> +}
> +
> +static PyObject *
> valpy_power (PyObject *self, PyObject *other, PyObject *unused)
> {
> /* We don't support the ternary form of pow. I don't know how to express
> @@ -1837,7 +1869,7 @@ static PyNumberMethods value_object_as_number = {
> NULL, /* nb_inplace_and */
> NULL, /* nb_inplace_xor */
> NULL, /* nb_inplace_or */
> - NULL, /* nb_floor_divide */
> + valpy_floordiv, /* nb_floor_divide */
> valpy_divide, /* nb_true_divide */
> NULL, /* nb_inplace_floor_divide */
> NULL, /* nb_inplace_true_divide */
> diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp
> index 57a9ba1..81837e9 100644
> --- a/gdb/testsuite/gdb.python/py-value.exp
> +++ b/gdb/testsuite/gdb.python/py-value.exp
> @@ -87,6 +87,8 @@ proc test_value_numeric_ops {} {
> gdb_test "python print ('result = ' + str(f/g))" " = 0.5" "divide two double values"
> gdb_test "python print ('result = ' + str(i%j))" " = 1" "take remainder of two integer values"
> # Remainder of float is implemented in Python but not in GDB's value system.
> + gdb_test "python print ('result = ' + str(i//j))" " = 2" "floor-divide two integer values"
> + gdb_test "python print ('result = ' + str(f//g))" " = 0" "floor-divide two double values"
Is the "two double values" test returning an integer somehow?
I ask because IIUC, regardless of Python version, a floor-divide
involving a float should result in a float, while a floor-divide of
integers should result in an integer. And that's what the patch looks
like should end up with. So I was expecting to see "0.0" in
the "two double values" case:
(gdb) python print (5.0//6.0)
0.0
(gdb) python print (5//6)
0
I think it'd be good to test with negative numbers too, to make
sure that we round (and keep rounding) toward the same
direction Python rounds:
(gdb) python print (8.0//-3)
-3.0
(gdb) python print (8//-3)
-3
(gdb) print 8/-3
$1 = -2
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Implement floordiv operator for gdb.Value
2016-09-20 15:41 ` Pedro Alves
@ 2016-09-20 16:41 ` Jonathan Wakely
2016-09-20 16:42 ` Jonathan Wakely
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Jonathan Wakely @ 2016-09-20 16:41 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 20/09/16 16:33 +0100, Pedro Alves wrote:
>Hi there,
>
>Thanks!
>
>On 09/20/2016 02:26 PM, Jonathan Wakely wrote:
>> This is my attempt to implement the // operator on gdb.Value objects.
>> There is already BINOP_INTDIV which works fine for integral types, but
>> for floats I use BINOP_DIV and then call floor() on the result. This
>> doesn't support decimal floats though.
>>
>> Is this a reasonable solution? Is the test sufficient?
>>
>
>See below.
>
>> @@ -1142,7 +1160,15 @@ valpy_binop_throw (enum valpy_opcode opcode, PyObject *self, PyObject *other)
>> }
>>
>> if (res_val)
>> - result = value_to_value_object (res_val);
>> + {
>> + if (floor_it)
>> + {
>> + double d = value_as_double (res_val);
>
>Should be s/double/DOUBLEST, I suppose?
OK - if I do that then floor(d) will convert it back to double,
unless you #include <cmath> and using std::floor, so that the overload
for long double is visible (in C++ <math.h> names like floor are
overloaded so you don't need to use floorf/floor/floorl according to
the type).
>> diff --git a/gdb/testsuite/gdb.python/py-value.exp b/gdb/testsuite/gdb.python/py-value.exp
>> index 57a9ba1..81837e9 100644
>> --- a/gdb/testsuite/gdb.python/py-value.exp
>> +++ b/gdb/testsuite/gdb.python/py-value.exp
>> @@ -87,6 +87,8 @@ proc test_value_numeric_ops {} {
>> gdb_test "python print ('result = ' + str(f/g))" " = 0.5" "divide two double values"
>> gdb_test "python print ('result = ' + str(i%j))" " = 1" "take remainder of two integer values"
>> # Remainder of float is implemented in Python but not in GDB's value system.
>> + gdb_test "python print ('result = ' + str(i//j))" " = 2" "floor-divide two integer values"
>> + gdb_test "python print ('result = ' + str(f//g))" " = 0" "floor-divide two double values"
>
>Is the "two double values" test returning an integer somehow?
>
>I ask because IIUC, regardless of Python version, a floor-divide
>involving a float should result in a float, while a floor-divide of
>integers should result in an integer. And that's what the patch looks
>like should end up with. So I was expecting to see "0.0" in
>the "two double values" case:
>
> (gdb) python print (5.0//6.0)
> 0.0
> (gdb) python print (5//6)
> 0
This seems to be an existing property of gdb.Value, as even using the
normal division operator (and without my patch) I see floats printed
without a decimal part when they are an integer value:
(gdb) python print (gdb.Value(5.0)/5.0)
1
(gdb) python print (5.0/5.0)
1.0
>I think it'd be good to test with negative numbers too, to make
>sure that we round (and keep rounding) toward the same
>direction Python rounds:
>
> (gdb) python print (8.0//-3)
> -3.0
> (gdb) python print (8//-3)
> -3
> (gdb) print 8/-3
> $1 = -2
Good point, I'll do that.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Implement floordiv operator for gdb.Value
2016-09-20 16:41 ` Jonathan Wakely
@ 2016-09-20 16:42 ` Jonathan Wakely
2016-09-20 17:01 ` Pedro Alves
2016-09-20 17:08 ` Paul.Koning
2 siblings, 0 replies; 10+ messages in thread
From: Jonathan Wakely @ 2016-09-20 16:42 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 20/09/16 17:35 +0100, Jonathan Wakely wrote:
>On 20/09/16 16:33 +0100, Pedro Alves wrote:
>>>@@ -1142,7 +1160,15 @@ valpy_binop_throw (enum valpy_opcode opcode, PyObject *self, PyObject *other)
>>> }
>>>
>>> if (res_val)
>>>- result = value_to_value_object (res_val);
>>>+ {
>>>+ if (floor_it)
>>>+ {
>>>+ double d = value_as_double (res_val);
>>
>>Should be s/double/DOUBLEST, I suppose?
>
>OK - if I do that then floor(d) will convert it back to double,
>unless you #include <cmath> and using std::floor, so that the overload
>for long double is visible (in C++ <math.h> names like floor are
>overloaded so you don't need to use floorf/floor/floorl according to
>the type).
P.S. In theory it should work with <math.h> and without the
using-declaration, but with GCC that wasn't true until GCC 6.1 (see
http://developerblog.redhat.com/2016/02/29/why-cstdlib-is-more-complicated-than-you-might-think/
for the gory details).
So to get the right result for older versions of GCC you need <cmath>.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Implement floordiv operator for gdb.Value
2016-09-20 16:41 ` Jonathan Wakely
2016-09-20 16:42 ` Jonathan Wakely
@ 2016-09-20 17:01 ` Pedro Alves
2016-09-20 17:11 ` Jonathan Wakely
2016-09-20 17:08 ` Paul.Koning
2 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2016-09-20 17:01 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: gdb-patches
On 09/20/2016 05:35 PM, Jonathan Wakely wrote:
> On 20/09/16 16:33 +0100, Pedro Alves wrote:
>> Hi there,
>>
>> Thanks!
>>
>> On 09/20/2016 02:26 PM, Jonathan Wakely wrote:
>>> This is my attempt to implement the // operator on gdb.Value objects.
>>> There is already BINOP_INTDIV which works fine for integral types, but
>>> for floats I use BINOP_DIV and then call floor() on the result. This
>>> doesn't support decimal floats though.
>>>
>>> Is this a reasonable solution? Is the test sufficient?
>>>
>>
>> See below.
>>
>>> @@ -1142,7 +1160,15 @@ valpy_binop_throw (enum valpy_opcode opcode,
>>> PyObject *self, PyObject *other)
>>> }
>>>
>>> if (res_val)
>>> - result = value_to_value_object (res_val);
>>> + {
>>> + if (floor_it)
>>> + {
>>> + double d = value_as_double (res_val);
>>
>> Should be s/double/DOUBLEST, I suppose?
>
> OK - if I do that then floor(d) will convert it back to double,
> unless you #include <cmath> and using std::floor, so that the overload
> for long double is visible (in C++ <math.h> names like floor are
> overloaded so you don't need to use floorf/floor/floorl according to
> the type).
OK. I remember reading your blog about this mess a while ago.
If easy to do, sounds like we should just do it. OOC, would calling
std::floor directly instead of using "using" work just as well?
(This kind of raises the question of which float type / format / representation
to use for arithmetic here -- host's or target's. gdb currently always
uses host's, but that's a much larger issue that we can just
continue to ignore.)
>> Is the "two double values" test returning an integer somehow?
>>
>> I ask because IIUC, regardless of Python version, a floor-divide
>> involving a float should result in a float, while a floor-divide of
>> integers should result in an integer. And that's what the patch looks
>> like should end up with. So I was expecting to see "0.0" in
>> the "two double values" case:
>>
>> (gdb) python print (5.0//6.0)
>> 0.0
>> (gdb) python print (5//6)
>> 0
>
> This seems to be an existing property of gdb.Value, as even using the
> normal division operator (and without my patch) I see floats printed
> without a decimal part when they are an integer value:
>
> (gdb) python print (gdb.Value(5.0)/5.0)
> 1
> (gdb) python print (5.0/5.0)
> 1.0
Curious. Off hand looks like a bug to me. But since it's orthogonal
to your patch, let's leave it.
>> I think it'd be good to test with negative numbers too, to make
>> sure that we round (and keep rounding) toward the same
>> direction Python rounds:
>>
>> (gdb) python print (8.0//-3)
>> -3.0
>> (gdb) python print (8//-3)
>> -3
>> (gdb) print 8/-3
>> $1 = -2
>
> Good point, I'll do that.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Implement floordiv operator for gdb.Value
2016-09-20 17:01 ` Pedro Alves
@ 2016-09-20 17:11 ` Jonathan Wakely
0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Wakely @ 2016-09-20 17:11 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 20/09/16 17:57 +0100, Pedro Alves wrote:
>On 09/20/2016 05:35 PM, Jonathan Wakely wrote:
>> On 20/09/16 16:33 +0100, Pedro Alves wrote:
>>> Hi there,
>>>
>>> Thanks!
>>>
>>> On 09/20/2016 02:26 PM, Jonathan Wakely wrote:
>>>> This is my attempt to implement the // operator on gdb.Value objects.
>>>> There is already BINOP_INTDIV which works fine for integral types, but
>>>> for floats I use BINOP_DIV and then call floor() on the result. This
>>>> doesn't support decimal floats though.
>>>>
>>>> Is this a reasonable solution? Is the test sufficient?
>>>>
>>>
>>> See below.
>>>
>>>> @@ -1142,7 +1160,15 @@ valpy_binop_throw (enum valpy_opcode opcode,
>>>> PyObject *self, PyObject *other)
>>>> }
>>>>
>>>> if (res_val)
>>>> - result = value_to_value_object (res_val);
>>>> + {
>>>> + if (floor_it)
>>>> + {
>>>> + double d = value_as_double (res_val);
>>>
>>> Should be s/double/DOUBLEST, I suppose?
>>
>> OK - if I do that then floor(d) will convert it back to double,
>> unless you #include <cmath> and using std::floor, so that the overload
>> for long double is visible (in C++ <math.h> names like floor are
>> overloaded so you don't need to use floorf/floor/floorl according to
>> the type).
>
>OK. I remember reading your blog about this mess a while ago.
>
>If easy to do, sounds like we should just do it. OOC, would calling
>std::floor directly instead of using "using" work just as well?
Yes, and that's exactly what I did locally a few minutes ago. Calling
std::floor explicitly probably makes it more obvious that it relies on
the C++ overloads to preserve the right type.
>(This kind of raises the question of which float type / format / representation
>to use for arithmetic here -- host's or target's. gdb currently always
>uses host's, but that's a much larger issue that we can just
>continue to ignore.)
I'm happy to ignore it :-)
>>> Is the "two double values" test returning an integer somehow?
>>>
>>> I ask because IIUC, regardless of Python version, a floor-divide
>>> involving a float should result in a float, while a floor-divide of
>>> integers should result in an integer. And that's what the patch looks
>>> like should end up with. So I was expecting to see "0.0" in
>>> the "two double values" case:
>>>
>>> (gdb) python print (5.0//6.0)
>>> 0.0
>>> (gdb) python print (5//6)
>>> 0
>>
>> This seems to be an existing property of gdb.Value, as even using the
>> normal division operator (and without my patch) I see floats printed
>> without a decimal part when they are an integer value:
>>
>> (gdb) python print (gdb.Value(5.0)/5.0)
>> 1
>> (gdb) python print (5.0/5.0)
>> 1.0
>
>Curious. Off hand looks like a bug to me. But since it's orthogonal
>to your patch, let's leave it.
>
>>> I think it'd be good to test with negative numbers too, to make
>>> sure that we round (and keep rounding) toward the same
>>> direction Python rounds:
>>>
>>> (gdb) python print (8.0//-3)
>>> -3.0
>>> (gdb) python print (8//-3)
>>> -3
>>> (gdb) print 8/-3
>>> $1 = -2
>>
>> Good point, I'll do that.
Definitely worth testing because my patch gives the wrong result:
(gdb) python print( gdb.Value(8) // gdb.Value(-3) )
-2
This is because I'm using the same BINOP_DIV operation as GDB's 8/-3
uses, but Python's // should round to -inf. I'll fix that.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Implement floordiv operator for gdb.Value
2016-09-20 16:41 ` Jonathan Wakely
2016-09-20 16:42 ` Jonathan Wakely
2016-09-20 17:01 ` Pedro Alves
@ 2016-09-20 17:08 ` Paul.Koning
2016-09-20 18:20 ` Jonathan Wakely
2 siblings, 1 reply; 10+ messages in thread
From: Paul.Koning @ 2016-09-20 17:08 UTC (permalink / raw)
To: jwakely; +Cc: palves, gdb-patches
> On Sep 20, 2016, at 12:35 PM, Jonathan Wakely <jwakely@redhat.com> wrote:
>
>> ...
>
> This seems to be an existing property of gdb.Value, as even using the
> normal division operator (and without my patch) I see floats printed
> without a decimal part when they are an integer value:
>
> (gdb) python print (gdb.Value(5.0)/5.0)
> 1
> (gdb) python print (5.0/5.0)
> 1.0
In all this, please keep in mind that this is one place where Python 2 and Python 3 differ:
$ python
Python 2.7.6 (v2.7.6:3a1db0d2747e, Nov 10 2013, 00:42:54)
[GCC 4.2.1 (Apple Inc. build 5666) (dot 3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> 1/2
0
>>> ^D
$ python3
Python 3.5.1 (v3.5.1:37a07cee5969, Dec 5 2015, 21:12:44)
[GCC 4.2.1 (Apple Inc. build 5666) (dot 3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> 1/2
0.5
>>> ^D
Python 2 can be told to do it the new way:
$ python2
Python 2.7.6 (v2.7.6:3a1db0d2747e, Nov 10 2013, 00:42:54)
[GCC 4.2.1 (Apple Inc. build 5666) (dot 3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from __future__ import division
>>> 1/2
0.5
>>>
paul
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Implement floordiv operator for gdb.Value
2016-09-20 17:08 ` Paul.Koning
@ 2016-09-20 18:20 ` Jonathan Wakely
2016-09-20 19:06 ` Paul.Koning
0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Wakely @ 2016-09-20 18:20 UTC (permalink / raw)
To: Paul.Koning; +Cc: palves, gdb-patches
On 20/09/16 17:00 +0000, Paul.Koning@dell.com wrote:
>
>> On Sep 20, 2016, at 12:35 PM, Jonathan Wakely <jwakely@redhat.com> wrote:
>>
>>> ...
>>
>> This seems to be an existing property of gdb.Value, as even using the
>> normal division operator (and without my patch) I see floats printed
>> without a decimal part when they are an integer value:
>>
>> (gdb) python print (gdb.Value(5.0)/5.0)
>> 1
>> (gdb) python print (5.0/5.0)
>> 1.0
>
>In all this, please keep in mind that this is one place where Python 2 and Python 3 differ:
Right, see https://sourceware.org/ml/gdb-patches/2016-09/msg00221.html
for a crazy idea that would make gdb.Value match the Python version
it's built against.
That wouldn't help for Python 2 using __future__ division though.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Implement floordiv operator for gdb.Value
2016-09-20 18:20 ` Jonathan Wakely
@ 2016-09-20 19:06 ` Paul.Koning
0 siblings, 0 replies; 10+ messages in thread
From: Paul.Koning @ 2016-09-20 19:06 UTC (permalink / raw)
To: jwakely; +Cc: palves, gdb-patches
> On Sep 20, 2016, at 1:11 PM, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On 20/09/16 17:00 +0000, Paul.Koning@dell.com wrote:
>>
>>> On Sep 20, 2016, at 12:35 PM, Jonathan Wakely <jwakely@redhat.com> wrote:
>>>
>>>> ...
>>>
>>> This seems to be an existing property of gdb.Value, as even using the
>>> normal division operator (and without my patch) I see floats printed
>>> without a decimal part when they are an integer value:
>>>
>>> (gdb) python print (gdb.Value(5.0)/5.0)
>>> 1
>>> (gdb) python print (5.0/5.0)
>>> 1.0
>>
>> In all this, please keep in mind that this is one place where Python 2 and Python 3 differ:
>
> Right, see https://sourceware.org/ml/gdb-patches/2016-09/msg00221.html
> for a crazy idea that would make gdb.Value match the Python version
> it's built against.
>
> That wouldn't help for Python 2 using __future__ division though.
Yes. You'd expect that this would be easy to find, but that isn't the case. It may be accessible from C code, if you can find the parser flags from where you are. Look for CO_FUTURE in the source tree.
paul
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-09-20 18:20 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-20 13:34 [PATCH] Implement floordiv operator for gdb.Value Jonathan Wakely
2016-09-20 14:46 ` Jonathan Wakely
2016-09-20 15:41 ` Pedro Alves
2016-09-20 16:41 ` Jonathan Wakely
2016-09-20 16:42 ` Jonathan Wakely
2016-09-20 17:01 ` Pedro Alves
2016-09-20 17:11 ` Jonathan Wakely
2016-09-20 17:08 ` Paul.Koning
2016-09-20 18:20 ` Jonathan Wakely
2016-09-20 19:06 ` Paul.Koning
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox