public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gdb python LineTable.source_lines docstring returns List, not FrozenSet
@ 2015-07-04 20:14 Ciro Santilli
  2015-07-14 19:01 ` Ciro Santilli
  2015-07-23 14:02 ` Joel Brobecker
  0 siblings, 2 replies; 6+ messages in thread
From: Ciro Santilli @ 2015-07-04 20:14 UTC (permalink / raw)
  To: gdb-patches

This can be seen at
https://github.com/cirosantilli/binutils-gdb/blob/2edf562a17871d32b814acda9ced29856d405d6c/gdb/python/py-linetable.c#L285:

    source_list = PyDict_Keys (source_dict);
    return source_list;

PyDict_Keys makes list, not a frozen set:
https://docs.python.org/3/c-api/dict.html#c.PyDict_Keys

The python.texi is correct and says List. I wish we could concentrate
all documentation on a single place to avoid such clashes (only texi
or only docstrings).

From 2edf562a17871d32b814acda9ced29856d405d6c Mon Sep 17 00:00:00 2001
From: Ciro Santilli <ciro.santilli@gmail.com>
Date: Sat, 4 Jul 2015 21:56:58 +0200
Subject: [PATCH] gdb python LineTable.source_lines docstring returns List, not
 FrozenSet

---
 gdb/python/py-linetable.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/python/py-linetable.c b/gdb/python/py-linetable.c
index 195a8b3..82b4cfc 100644
--- a/gdb/python/py-linetable.c
+++ b/gdb/python/py-linetable.c
@@ -227,8 +227,8 @@ ltpy_has_line (PyObject *self, PyObject *args)
   Py_RETURN_FALSE;
 }

-/* Implementation of gdb.LineTable.source_lines (self) -> FrozenSet.
-   Returns a Python FrozenSet that contains source line entries in the
+/* Implementation of gdb.LineTable.source_lines (self) -> List.
+   Returns a Python List that contains source line entries in the
    line table.  This function will just return the source lines
    without corresponding addresses.  */

@@ -488,8 +488,8 @@ Return executable locations for a given source line." },
     "has_line (lineno) -> Boolean\n\
 Return TRUE if this line has executable information, FALSE if not." },
   { "source_lines", ltpy_get_all_source_lines, METH_NOARGS,
-    "source_lines () -> FrozenSet\n\
-Return a frozen set of all executable source lines." },
+    "source_lines () -> List\n\
+Return a list of all executable source lines." },
   { "is_valid", ltpy_is_valid, METH_NOARGS,
     "is_valid () -> Boolean.\n\
 Return True if this Linetable is valid, False if not." },
-- 
1.9.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdb python LineTable.source_lines docstring returns List, not FrozenSet
  2015-07-04 20:14 [PATCH] gdb python LineTable.source_lines docstring returns List, not FrozenSet Ciro Santilli
@ 2015-07-14 19:01 ` Ciro Santilli
  2015-07-22  7:48   ` Ciro Santilli
  2015-07-23 14:02 ` Joel Brobecker
  1 sibling, 1 reply; 6+ messages in thread
From: Ciro Santilli @ 2015-07-14 19:01 UTC (permalink / raw)
  To: gdb-patches, tromey

Ping.

Tromey, taking the liberty of pinging you directly because you seem
like "the Python guy".

Let me know if there is someone or a method more appropriate to ping /
submit for this type of patch.

Cheers.

On Sat, Jul 4, 2015 at 10:14 PM, Ciro Santilli <ciro.santilli@gmail.com> wrote:
> This can be seen at
> https://github.com/cirosantilli/binutils-gdb/blob/2edf562a17871d32b814acda9ced29856d405d6c/gdb/python/py-linetable.c#L285:
>
>     source_list = PyDict_Keys (source_dict);
>     return source_list;
>
> PyDict_Keys makes list, not a frozen set:
> https://docs.python.org/3/c-api/dict.html#c.PyDict_Keys
>
> The python.texi is correct and says List. I wish we could concentrate
> all documentation on a single place to avoid such clashes (only texi
> or only docstrings).
>
> From 2edf562a17871d32b814acda9ced29856d405d6c Mon Sep 17 00:00:00 2001
> From: Ciro Santilli <ciro.santilli@gmail.com>
> Date: Sat, 4 Jul 2015 21:56:58 +0200
> Subject: [PATCH] gdb python LineTable.source_lines docstring returns List, not
>  FrozenSet
>
> ---
>  gdb/python/py-linetable.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/python/py-linetable.c b/gdb/python/py-linetable.c
> index 195a8b3..82b4cfc 100644
> --- a/gdb/python/py-linetable.c
> +++ b/gdb/python/py-linetable.c
> @@ -227,8 +227,8 @@ ltpy_has_line (PyObject *self, PyObject *args)
>    Py_RETURN_FALSE;
>  }
>
> -/* Implementation of gdb.LineTable.source_lines (self) -> FrozenSet.
> -   Returns a Python FrozenSet that contains source line entries in the
> +/* Implementation of gdb.LineTable.source_lines (self) -> List.
> +   Returns a Python List that contains source line entries in the
>     line table.  This function will just return the source lines
>     without corresponding addresses.  */
>
> @@ -488,8 +488,8 @@ Return executable locations for a given source line." },
>      "has_line (lineno) -> Boolean\n\
>  Return TRUE if this line has executable information, FALSE if not." },
>    { "source_lines", ltpy_get_all_source_lines, METH_NOARGS,
> -    "source_lines () -> FrozenSet\n\
> -Return a frozen set of all executable source lines." },
> +    "source_lines () -> List\n\
> +Return a list of all executable source lines." },
>    { "is_valid", ltpy_is_valid, METH_NOARGS,
>      "is_valid () -> Boolean.\n\
>  Return True if this Linetable is valid, False if not." },
> --
> 1.9.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdb python LineTable.source_lines docstring returns List, not FrozenSet
  2015-07-14 19:01 ` Ciro Santilli
@ 2015-07-22  7:48   ` Ciro Santilli
  0 siblings, 0 replies; 6+ messages in thread
From: Ciro Santilli @ 2015-07-22  7:48 UTC (permalink / raw)
  To: gdb-patches

Ping 2. Let me know your desired ping frequency, currently weekly as
per Tromey's suggestion.

On Tue, Jul 14, 2015 at 9:01 PM, Ciro Santilli <ciro.santilli@gmail.com> wrote:
> Ping.
>
> Tromey, taking the liberty of pinging you directly because you seem
> like "the Python guy".
>
> Let me know if there is someone or a method more appropriate to ping /
> submit for this type of patch.
>
> Cheers.
>
> On Sat, Jul 4, 2015 at 10:14 PM, Ciro Santilli <ciro.santilli@gmail.com> wrote:
>> This can be seen at
>> https://github.com/cirosantilli/binutils-gdb/blob/2edf562a17871d32b814acda9ced29856d405d6c/gdb/python/py-linetable.c#L285:
>>
>>     source_list = PyDict_Keys (source_dict);
>>     return source_list;
>>
>> PyDict_Keys makes list, not a frozen set:
>> https://docs.python.org/3/c-api/dict.html#c.PyDict_Keys
>>
>> The python.texi is correct and says List. I wish we could concentrate
>> all documentation on a single place to avoid such clashes (only texi
>> or only docstrings).
>>
>> From 2edf562a17871d32b814acda9ced29856d405d6c Mon Sep 17 00:00:00 2001
>> From: Ciro Santilli <ciro.santilli@gmail.com>
>> Date: Sat, 4 Jul 2015 21:56:58 +0200
>> Subject: [PATCH] gdb python LineTable.source_lines docstring returns List, not
>>  FrozenSet
>>
>> ---
>>  gdb/python/py-linetable.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/gdb/python/py-linetable.c b/gdb/python/py-linetable.c
>> index 195a8b3..82b4cfc 100644
>> --- a/gdb/python/py-linetable.c
>> +++ b/gdb/python/py-linetable.c
>> @@ -227,8 +227,8 @@ ltpy_has_line (PyObject *self, PyObject *args)
>>    Py_RETURN_FALSE;
>>  }
>>
>> -/* Implementation of gdb.LineTable.source_lines (self) -> FrozenSet.
>> -   Returns a Python FrozenSet that contains source line entries in the
>> +/* Implementation of gdb.LineTable.source_lines (self) -> List.
>> +   Returns a Python List that contains source line entries in the
>>     line table.  This function will just return the source lines
>>     without corresponding addresses.  */
>>
>> @@ -488,8 +488,8 @@ Return executable locations for a given source line." },
>>      "has_line (lineno) -> Boolean\n\
>>  Return TRUE if this line has executable information, FALSE if not." },
>>    { "source_lines", ltpy_get_all_source_lines, METH_NOARGS,
>> -    "source_lines () -> FrozenSet\n\
>> -Return a frozen set of all executable source lines." },
>> +    "source_lines () -> List\n\
>> +Return a list of all executable source lines." },
>>    { "is_valid", ltpy_is_valid, METH_NOARGS,
>>      "is_valid () -> Boolean.\n\
>>  Return True if this Linetable is valid, False if not." },
>> --
>> 1.9.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdb python LineTable.source_lines docstring returns List, not FrozenSet
  2015-07-04 20:14 [PATCH] gdb python LineTable.source_lines docstring returns List, not FrozenSet Ciro Santilli
  2015-07-14 19:01 ` Ciro Santilli
@ 2015-07-23 14:02 ` Joel Brobecker
  2015-07-23 19:31   ` Ciro Santilli
  1 sibling, 1 reply; 6+ messages in thread
From: Joel Brobecker @ 2015-07-23 14:02 UTC (permalink / raw)
  To: Ciro Santilli; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1538 bytes --]

Sorry for the delay in reviewing this. Thanks for the patch.

Yes, weekly pings are absolutely appropriate. Thanks for the reminders!

> >From 2edf562a17871d32b814acda9ced29856d405d6c Mon Sep 17 00:00:00 2001
> From: Ciro Santilli <ciro.santilli@gmail.com>
> Date: Sat, 4 Jul 2015 21:56:58 +0200
> Subject: [PATCH] gdb python LineTable.source_lines docstring returns List, not
>  FrozenSet

Overall, the patch looked good, and I just pushed it after creating
a proper revision log and doing a round of testing (our penance
for letting your patch sit for a while).

But if you are going to send more patches for inclusion, we'll need
you to have a copyright assignment on file. If you want to start
the process, let me know, and I will send you the form to fill.
For now, this patch was small and obvious enough that copyright
was not an issue.

Also, we have a fairly detailed procedure for submitting patches.
We have a checklist:

    https://sourceware.org/gdb/wiki/ContributionChecklist

Once you have a copyright assignment on file, and your patch
submissions start following the procedure above, we can give you
write-after-approval access to the GDB repository.

Attached is the patch as I pushed it...

gdb/ChangeLog:

        * py-linetable.c (ltpy_get_all_source_lines): Adjust function
        documentation to say that it returns a list rather than
        a FrozenSet.
        (linetable_object_methods): Update the docstring of the
        "source_line" entry.

Tested on x86_64-linux.

Thanks again for the patch.
-- 
Joel

[-- Attachment #2: 0001-py-linetable.c-Fix-doc-of-LineTable.source_lines-ret.patch --]
[-- Type: text/x-diff, Size: 2726 bytes --]

From 7d2f382045634a5ebed0b89f0c5e6e1e130939f7 Mon Sep 17 00:00:00 2001
From: Ciro Santilli <ciro.santilli@gmail.com>
Date: Thu, 23 Jul 2015 06:36:25 -0700
Subject: [PATCH] py-linetable.c: Fix doc of LineTable.source_lines' return
 type

The ltpy_get_all_source_lines function, use to implement
the gdb.LineTable.source_lines method, returns a list:

    source_list = PyDict_Keys (source_dict);
    return source_list;

This patch fixes the function's documentation as well as its docstring
to say that it returns a list rather than a FrozenSet.

gdb/ChangeLog:

        * py-linetable.c (ltpy_get_all_source_lines): Adjust function
        documentation to say that it returns a list rather than
        a FrozenSet.
        (linetable_object_methods): Update the docstring of the
        "source_line" entry.

Tested on x86_64-linux.
---
 gdb/ChangeLog             | 8 ++++++++
 gdb/python/py-linetable.c | 8 ++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 78096ab..ecc7f55 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2015-07-23  Ciro Santilli  <ciro.santilli@gmail.com>  (tiny patch)
+
+	* py-linetable.c (ltpy_get_all_source_lines): Adjust function
+	documentation to say that it returns a list rather than
+	a FrozenSet.
+	(linetable_object_methods): Update the docstring of the
+	"source_line" entry.
+
 2015-07-23  Pierre-Marie de Rodat  <derodat@adacore.com>
 
 	* gdbtypes.c (resolve_dynamic_array): Pass the peeled element
diff --git a/gdb/python/py-linetable.c b/gdb/python/py-linetable.c
index 195a8b3..82b4cfc 100644
--- a/gdb/python/py-linetable.c
+++ b/gdb/python/py-linetable.c
@@ -227,8 +227,8 @@ ltpy_has_line (PyObject *self, PyObject *args)
   Py_RETURN_FALSE;
 }
 
-/* Implementation of gdb.LineTable.source_lines (self) -> FrozenSet.
-   Returns a Python FrozenSet that contains source line entries in the
+/* Implementation of gdb.LineTable.source_lines (self) -> List.
+   Returns a Python List that contains source line entries in the
    line table.  This function will just return the source lines
    without corresponding addresses.  */
 
@@ -488,8 +488,8 @@ Return executable locations for a given source line." },
     "has_line (lineno) -> Boolean\n\
 Return TRUE if this line has executable information, FALSE if not." },
   { "source_lines", ltpy_get_all_source_lines, METH_NOARGS,
-    "source_lines () -> FrozenSet\n\
-Return a frozen set of all executable source lines." },
+    "source_lines () -> List\n\
+Return a list of all executable source lines." },
   { "is_valid", ltpy_is_valid, METH_NOARGS,
     "is_valid () -> Boolean.\n\
 Return True if this Linetable is valid, False if not." },
-- 
2.1.4


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdb python LineTable.source_lines docstring returns List, not FrozenSet
  2015-07-23 14:02 ` Joel Brobecker
@ 2015-07-23 19:31   ` Ciro Santilli
  2015-07-30 12:29     ` Ciro Santilli
  0 siblings, 1 reply; 6+ messages in thread
From: Ciro Santilli @ 2015-07-23 19:31 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

Thanks!

No worries: just checking that I'm following the usual group rules.

I know that patches take time to review ;-)

I will try to follow the rules more closely.

About the copyright assignment, I'd like to do it, I will likely
submit a few more small patches. How to proceed?

I wish FSF would open the GPG option outside of the US... :-)

On Thu, Jul 23, 2015 at 4:02 PM, Joel Brobecker <brobecker@adacore.com> wrote:
> Sorry for the delay in reviewing this. Thanks for the patch.
>
> Yes, weekly pings are absolutely appropriate. Thanks for the reminders!
>
>> >From 2edf562a17871d32b814acda9ced29856d405d6c Mon Sep 17 00:00:00 2001
>> From: Ciro Santilli <ciro.santilli@gmail.com>
>> Date: Sat, 4 Jul 2015 21:56:58 +0200
>> Subject: [PATCH] gdb python LineTable.source_lines docstring returns List, not
>>  FrozenSet
>
> Overall, the patch looked good, and I just pushed it after creating
> a proper revision log and doing a round of testing (our penance
> for letting your patch sit for a while).
>
> But if you are going to send more patches for inclusion, we'll need
> you to have a copyright assignment on file. If you want to start
> the process, let me know, and I will send you the form to fill.
> For now, this patch was small and obvious enough that copyright
> was not an issue.
>
> Also, we have a fairly detailed procedure for submitting patches.
> We have a checklist:
>
>     https://sourceware.org/gdb/wiki/ContributionChecklist
>
> Once you have a copyright assignment on file, and your patch
> submissions start following the procedure above, we can give you
> write-after-approval access to the GDB repository.
>
> Attached is the patch as I pushed it...
>
> gdb/ChangeLog:
>
>         * py-linetable.c (ltpy_get_all_source_lines): Adjust function
>         documentation to say that it returns a list rather than
>         a FrozenSet.
>         (linetable_object_methods): Update the docstring of the
>         "source_line" entry.
>
> Tested on x86_64-linux.
>
> Thanks again for the patch.
> --
> Joel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdb python LineTable.source_lines docstring returns List, not FrozenSet
  2015-07-23 19:31   ` Ciro Santilli
@ 2015-07-30 12:29     ` Ciro Santilli
  0 siblings, 0 replies; 6+ messages in thread
From: Ciro Santilli @ 2015-07-30 12:29 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

Ping.

On Thu, Jul 23, 2015 at 9:31 PM, Ciro Santilli <ciro.santilli@gmail.com> wrote:
> Thanks!
>
> No worries: just checking that I'm following the usual group rules.
>
> I know that patches take time to review ;-)
>
> I will try to follow the rules more closely.
>
> About the copyright assignment, I'd like to do it, I will likely
> submit a few more small patches. How to proceed?
>
> I wish FSF would open the GPG option outside of the US... :-)
>
> On Thu, Jul 23, 2015 at 4:02 PM, Joel Brobecker <brobecker@adacore.com> wrote:
>> Sorry for the delay in reviewing this. Thanks for the patch.
>>
>> Yes, weekly pings are absolutely appropriate. Thanks for the reminders!
>>
>>> >From 2edf562a17871d32b814acda9ced29856d405d6c Mon Sep 17 00:00:00 2001
>>> From: Ciro Santilli <ciro.santilli@gmail.com>
>>> Date: Sat, 4 Jul 2015 21:56:58 +0200
>>> Subject: [PATCH] gdb python LineTable.source_lines docstring returns List, not
>>>  FrozenSet
>>
>> Overall, the patch looked good, and I just pushed it after creating
>> a proper revision log and doing a round of testing (our penance
>> for letting your patch sit for a while).
>>
>> But if you are going to send more patches for inclusion, we'll need
>> you to have a copyright assignment on file. If you want to start
>> the process, let me know, and I will send you the form to fill.
>> For now, this patch was small and obvious enough that copyright
>> was not an issue.
>>
>> Also, we have a fairly detailed procedure for submitting patches.
>> We have a checklist:
>>
>>     https://sourceware.org/gdb/wiki/ContributionChecklist
>>
>> Once you have a copyright assignment on file, and your patch
>> submissions start following the procedure above, we can give you
>> write-after-approval access to the GDB repository.
>>
>> Attached is the patch as I pushed it...
>>
>> gdb/ChangeLog:
>>
>>         * py-linetable.c (ltpy_get_all_source_lines): Adjust function
>>         documentation to say that it returns a list rather than
>>         a FrozenSet.
>>         (linetable_object_methods): Update the docstring of the
>>         "source_line" entry.
>>
>> Tested on x86_64-linux.
>>
>> Thanks again for the patch.
>> --
>> Joel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-07-30 12:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-04 20:14 [PATCH] gdb python LineTable.source_lines docstring returns List, not FrozenSet Ciro Santilli
2015-07-14 19:01 ` Ciro Santilli
2015-07-22  7:48   ` Ciro Santilli
2015-07-23 14:02 ` Joel Brobecker
2015-07-23 19:31   ` Ciro Santilli
2015-07-30 12:29     ` Ciro Santilli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).