public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@embecosm.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>,
	 Simon Marchi <simon.marchi@polymtl.ca>,
	Tom Tromey <tom@tromey.com>,  Simon Sobisch <simonsobisch@web.de>
Subject: [PATCH v6 5/8] GDB/Python: Use None for `var_zuinteger_unlimited' value set to `unlimited'
Date: Wed, 17 Aug 2022 23:04:38 +0100 (BST)	[thread overview]
Message-ID: <alpine.DEB.2.20.2208171751060.10833@tpp.orcam.me.uk> (raw)
In-Reply-To: <alpine.DEB.2.20.2208151502460.10833@tpp.orcam.me.uk>

Consistently with parameters of the PARAM_UINTEGER and PARAM_INTEGER 
types return the special value of `None' for a PARAM_ZUINTEGER_UNLIMITED 
parameter set to `unlimited', fixing an inconsistency introduced with 
commit 0489430a0e1a ("Handle var_zuinteger and var_zuinteger_unlimited 
from Python"); cf. PR python/20084.  Adjust the testsuite accordingly.

This makes all the three parameter types consistent with each other as 
far as the use of `None' is concerned, and similar to the Guile/Scheme 
interface where the `#:unlimited' keyword is likewise used.  We have a 
precedent already documented for a similar API correction:

 -- Function: gdb.breakpoints ()
     Return a sequence holding all of GDB's breakpoints.  *Note
     Breakpoints In Python::, for more information.  In GDB version 7.11
     and earlier, this function returned 'None' if there were no
     breakpoints.  This peculiarity was subsequently fixed, and now
     'gdb.breakpoints' returns an empty sequence in this case.

made in the past.

And then we have documentation not matching the interface as far as the 
value of `None' already returned for parameters of the PARAM_UINTEGER 
and PARAM_INTEGER types is concerned, and the case of an incorrect 
assertion for PARAM_UINTEGER and PARAM_ZUINTEGER_UNLIMITED parameters in 
the sibling Guile/Scheme module making such parameters unusable that has 
never been reported, both indicating that these features may indeed not 
be heavily used, and therefore that the risk from such an API change is 
likely lower than the long-term burden of the inconsistency.

And where the value read from a parameter is only used for presentation 
purposes, then such a change is expected to be transparent.
---
Changes from v5:

- Refer to Python parameter types in the change description rather than
  underlying GDB variable types in preparation for breaking the tight 
  coupling between the two later in this series.

- Document existing and updated semantics in the GDB manual.

- Update the testsuite adjustment to fit in the now expanded test case.

- Add a NEWS entry.

No change from v4.

New change in v4.
---
 gdb/NEWS                                  |    7 +++++++
 gdb/doc/python.texi                       |   30 +++++++++++++++++++-----------
 gdb/python/python.c                       |   10 +++++++++-
 gdb/testsuite/gdb.python/py-parameter.exp |   16 +++++-----------
 4 files changed, 40 insertions(+), 23 deletions(-)

Index: src/gdb/NEWS
===================================================================
--- src.orig/gdb/NEWS
+++ src/gdb/NEWS
@@ -175,6 +175,13 @@ GNU/Linux/LoongArch (gdbserver)	loongarc
      gdb.BreakpointLocation objects specifying the locations where the
      breakpoint is inserted into the debuggee.
 
+  ** Parameters of gdb.PARAM_ZUINTEGER_UNLIMITED type now return the
+     value of None for the 'unlimited' setting, consistently with
+     parameters of gdb.PARAM_UINTEGER and gdb.PARAM_INTEGER types.
+     Parameters of all the three types now accept the value of None
+     to mean 'unlimited'.  The use of internal integer representation
+     for the 'unlimited' setting is now deprecated.
+
 * New features in the GDB remote stub, GDBserver
 
   ** GDBserver is now supported on LoongArch GNU/Linux.
Index: src/gdb/doc/python.texi
===================================================================
--- src.orig/gdb/doc/python.texi
+++ src/gdb/doc/python.texi
@@ -4598,14 +4598,16 @@ Python, true and false are represented u
 @findex PARAM_UINTEGER
 @findex gdb.PARAM_UINTEGER
 @item gdb.PARAM_UINTEGER
-The value is an unsigned integer.  The value of 0 should be
-interpreted to mean ``unlimited''.
+The value is an unsigned integer.  The value of @code{None} should be
+interpreted to mean ``unlimited'', and the value of 0 is reserved and
+should not be used.
 
 @findex PARAM_INTEGER
 @findex gdb.PARAM_INTEGER
 @item gdb.PARAM_INTEGER
-The value is a signed integer.  The value of 0 should be interpreted
-to mean ``unlimited''.
+The value is a signed integer.  The value of @code{None} should be
+interpreted to mean ``unlimited'', and the value of 0 is reserved and
+should not be used.
 
 @findex PARAM_STRING
 @findex gdb.PARAM_STRING
@@ -4635,21 +4637,27 @@ The value is a filename.  This is just l
 @findex PARAM_ZINTEGER
 @findex gdb.PARAM_ZINTEGER
 @item gdb.PARAM_ZINTEGER
-The value is an integer.  This is like @code{PARAM_INTEGER}, except 0
-is interpreted as itself.
+The value is a signed integer.  This is like @code{PARAM_INTEGER},
+except that 0 is allowed and the value of @code{None} is not supported.
 
 @findex PARAM_ZUINTEGER
 @findex gdb.PARAM_ZUINTEGER
 @item gdb.PARAM_ZUINTEGER
-The value is an unsigned integer.  This is like @code{PARAM_INTEGER},
-except 0 is interpreted as itself, and the value cannot be negative.
+The value is an unsigned integer.  This is like @code{PARAM_UINTEGER},
+except that 0 is allowed and the value of @code{None} is not supported.
 
 @findex PARAM_ZUINTEGER_UNLIMITED
 @findex gdb.PARAM_ZUINTEGER_UNLIMITED
 @item gdb.PARAM_ZUINTEGER_UNLIMITED
-The value is a signed integer.  This is like @code{PARAM_ZUINTEGER},
-except the special value -1 should be interpreted to mean
-``unlimited''.  Other negative values are not allowed.
+The value is a signed integer.  This is like @code{PARAM_INTEGER}
+including that the value of @code{None} should be interpreted to mean
+``unlimited'', except that 0 is allowed, and the value cannot be negative.
+
+In GDB version 12 and earlier, a parameter of this type when read would
+return -1 rather than @code{None} for the setting of ``unlimited''.
+This peculiarity was subsequently fixed, for consistency with parameters
+of @code{PARAM_UINTEGER} and @code{PARAM_INTEGER} types, so that all the
+three types return the value of @code{None} for ``unlimited''.
 
 @findex PARAM_ENUM
 @findex gdb.PARAM_ENUM
Index: src/gdb/python/python.c
===================================================================
--- src.orig/gdb/python/python.c
+++ src/gdb/python/python.c
@@ -509,9 +509,17 @@ gdbpy_parameter_value (const setting &va
 	Py_RETURN_NONE;
       /* Fall through.  */
     case var_zinteger:
-    case var_zuinteger_unlimited:
       return gdb_py_object_from_longest (var.get<int> ()).release ();
 
+    case var_zuinteger_unlimited:
+      {
+	int val = var.get<int> ();
+
+	if (val == -1)
+	  Py_RETURN_NONE;
+	return gdb_py_object_from_longest (val).release ();
+      }
+
     case var_uinteger:
       {
 	unsigned int val = var.get<unsigned int> ();
Index: src/gdb/testsuite/gdb.python/py-parameter.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.python/py-parameter.exp
+++ src/gdb/testsuite/gdb.python/py-parameter.exp
@@ -346,22 +346,16 @@ proc_with_prefix test_gdb_parameter { }
 	    "listsize" {
 		set param_get_zero None
 		set param_get_minus_one -1
-		set param_get_none None
-		set param_get_unlimited None
 		set param_set_minus_one ""
 	    }
 	    "print elements" {
 		set param_get_zero None
 		set param_get_minus_one None
-		set param_get_none None
-		set param_get_unlimited None
 		set param_set_minus_one $param_range_error
 	    }
 	    "max-completions" {
 		set param_get_zero 0
-		set param_get_minus_one -1
-		set param_get_none -1
-		set param_get_unlimited -1
+		set param_get_minus_one None
 		set param_set_minus_one ""
 	    }
 	    default {
@@ -392,13 +386,13 @@ proc_with_prefix test_gdb_parameter { }
 	    "test set to None"
 
 	gdb_test "python print(gdb.parameter('$param'))" \
-	    $param_get_none "test value of None"
+	    None "test value of None"
 
 	gdb_test_no_output "python gdb.set_parameter('$param', 'unlimited')" \
 	    "test set to 'unlimited'"
 
 	gdb_test "python print(gdb.parameter('$param'))" \
-	    $param_get_unlimited "test value of 'unlimited'"
+	    None "test value of 'unlimited'"
     }
 
     clean_restart
@@ -468,9 +462,9 @@ proc_with_prefix test_integer_parameter
 	    }
 	    PARAM_ZUINTEGER_UNLIMITED {
 		set param_get_zero 0
-		set param_get_minus_one -1
+		set param_get_minus_one None
 		set param_get_minus_five 1
-		set param_get_none -1
+		set param_get_none None
 		set param_set_minus_one ""
 		set param_set_minus_five $param_range_error
 		set param_set_none ""

  parent reply	other threads:[~2022-08-17 22:04 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-17 22:03 [PATCH v6 0/8] gdb: split array and string limiting options Maciej W. Rozycki
2022-08-17 22:03 ` [PATCH v6 1/8] GDB/Guile: Don't assert that an integer value is boolean Maciej W. Rozycki
2022-10-17 13:43   ` Simon Marchi
2022-10-21  7:58     ` Maciej W. Rozycki
2022-10-21 18:44   ` Simon Marchi
2022-10-21 20:54     ` Maciej W. Rozycki
2022-10-22  0:48       ` Simon Marchi
2022-08-17 22:03 ` [PATCH v6 2/8] GDB/doc: Document the Guile `#:unlimited' keyword Maciej W. Rozycki
2022-08-18  6:06   ` Eli Zaretskii
2022-09-01 10:31     ` Maciej W. Rozycki
2022-08-17 22:04 ` [PATCH v6 3/8] GDB/testsuite: Expand Python integer parameter coverage across all types Maciej W. Rozycki
2022-10-17 13:56   ` Simon Marchi
2022-10-21  7:59     ` Maciej W. Rozycki
2022-08-17 22:04 ` [PATCH v6 4/8] GDB/Python: Make `None' stand for `unlimited' in setting integer parameters Maciej W. Rozycki
2022-10-17 14:26   ` Simon Marchi
2022-10-21  8:03     ` Maciej W. Rozycki
2022-08-17 22:04 ` Maciej W. Rozycki [this message]
2022-08-18  6:08   ` [PATCH v6 5/8] GDB/Python: Use None for `var_zuinteger_unlimited' value set to `unlimited' Eli Zaretskii
2022-10-17 15:02   ` Simon Marchi
2022-10-29 15:58     ` Maciej W. Rozycki
2022-10-31 13:00       ` Simon Marchi
2022-10-31 13:31         ` Maciej W. Rozycki
2022-11-01 12:28           ` Maciej W. Rozycki
2022-10-26 11:58   ` Luis Machado
2022-10-29 13:52     ` Maciej W. Rozycki
2022-10-31  8:14       ` Luis Machado
2022-10-31 12:37         ` Luis Machado
2022-10-31 13:08           ` Maciej W. Rozycki
2022-10-31 13:14             ` Luis Machado
2022-10-31 14:05               ` Maciej W. Rozycki
2022-08-17 22:04 ` [PATCH v6 6/8] GDB: Allow arbitrary keywords in integer set commands Maciej W. Rozycki
2022-08-17 22:05 ` [PATCH v6 7/8] GDB: Add a character string limiting option Maciej W. Rozycki
2022-08-17 22:05 ` [PATCH v6 8/8] GDB/testsuite: Expand for character string limiting options Maciej W. Rozycki
2022-08-18  0:07   ` [PATCH v6.1 " Maciej W. Rozycki
2022-09-01 10:32 ` [PING][PATCH v6 0/8] gdb: split array and " Maciej W. Rozycki
2022-09-08  9:37 ` [PING^2][PATCH " Maciej W. Rozycki
2022-09-14 17:43 ` [PING^3][PATCH " Maciej W. Rozycki
2022-09-22 22:07 ` [PING^4][PATCH " Maciej W. Rozycki
2022-09-29  7:09 ` [PING^5][PATCH " Maciej W. Rozycki
2022-09-29  7:12   ` Simon Sobisch
2022-10-06 15:46 ` [PING^6][PATCH " Maciej W. Rozycki
2022-10-12 21:19 ` [PING^7][PATCH " Maciej W. Rozycki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.20.2208171751060.10833@tpp.orcam.me.uk \
    --to=macro@embecosm.com \
    --cc=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=simon.marchi@polymtl.ca \
    --cc=simonsobisch@web.de \
    --cc=tom@tromey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).