[RFC] Python's gdb.FRAME_UNWIND_NULL_ID is no longer used. What to do with it?

[RFC] Python's gdb.FRAME_UNWIND_NULL_ID is no longer used. What to do with it?

[Pedro Alves]

I'd assume scripts just check the result of Frame.unwind_stop_reason,
and compare it to gdb.FRAME_UNWIND_NO_REASON.  That at most, they'll
pass the result of Frame.unwind_stop_reason to
gdb.frame_stop_reason_string.  I'd prefer to just get rid of it, but
it may be best to keep this around for compatibility, in case a script
does refer to gdb.FRAME_UNWIND_NULL_ID directly.

In general, what's the policy for exposed constants like this in
Python?

gdb/
2013-11-28  Pedro Alves  <palves@redhat.com>

	* unwind_stop_reasons.def (UNWIND_NULL_ID): Update comment.

gdb/doc/
2013-11-28  Pedro Alves  <palves@redhat.com>

	* gdb.texinfo (Frames In Python) <gdb.FRAME_UNWIND_NULL_ID>:
	Update comment.
---
 gdb/doc/gdb.texinfo         |  4 +++-
 gdb/unwind_stop_reasons.def | 10 +++-------
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 004c376..ddb4b38 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26556,7 +26556,9 @@ function to a string. The value can be one of:
 No particular reason (older frames should be available).
 
 @item gdb.FRAME_UNWIND_NULL_ID
-The previous frame's analyzer returns an invalid result.
+The previous frame's analyzer returns an invalid result.  This is no
+longer used by @value{GDBN}, and is kept only for backward
+compatibility.
 
 @item gdb.FRAME_UNWIND_OUTERMOST
 This frame is the outermost.
diff --git a/gdb/unwind_stop_reasons.def b/gdb/unwind_stop_reasons.def
index ca5a74a..2c3d341 100644
--- a/gdb/unwind_stop_reasons.def
+++ b/gdb/unwind_stop_reasons.def
@@ -31,13 +31,9 @@
    or we didn't fail.  */
 SET (UNWIND_NO_REASON, "no reason")
 
-/* The previous frame's analyzer returns an invalid result
-   from this_id.
-
-   FIXME drow/2006-08-16: This is how GDB used to indicate end of
-   stack.  We should migrate to a model where frames always have a
-   valid ID, and this becomes not just an error but an internal
-   error.  But that's a project for another day.  */
+/* This is no longer used anywhere, but it's kept because it's exposed
+   to Python.  This is how GDB used to indicate end of stack.  We've
+   now migrated to a model where frames always have a valid ID.  */
 SET (UNWIND_NULL_ID, "unwinder did not report frame ID")
 
 /* This frame is the outermost.  */
-- 
1.7.11.7

Re: [RFC] Python's gdb.FRAME_UNWIND_NULL_ID is no longer used. What to do with it?

[Eli Zaretskii]

> From: Pedro Alves <palves@redhat.com>
> Date: Thu, 28 Nov 2013 18:45:56 +0000
> 
> I'd assume scripts just check the result of Frame.unwind_stop_reason,
> and compare it to gdb.FRAME_UNWIND_NO_REASON.  That at most, they'll
> pass the result of Frame.unwind_stop_reason to
> gdb.frame_stop_reason_string.  I'd prefer to just get rid of it, but
> it may be best to keep this around for compatibility, in case a script
> does refer to gdb.FRAME_UNWIND_NULL_ID directly.
> 
> In general, what's the policy for exposed constants like this in
> Python?
> 
> gdb/
> 2013-11-28  Pedro Alves  <palves@redhat.com>
> 
> 	* unwind_stop_reasons.def (UNWIND_NULL_ID): Update comment.
> 
> gdb/doc/
> 2013-11-28  Pedro Alves  <palves@redhat.com>
> 
> 	* gdb.texinfo (Frames In Python) <gdb.FRAME_UNWIND_NULL_ID>:
> 	Update comment.

OK for the documentation part.

Thanks.

Re: [RFC] Python's gdb.FRAME_UNWIND_NULL_ID is no longer used. What to do with it?

[Phil Muldoon]

On 28/11/13 18:45, Pedro Alves wrote:
> I'd assume scripts just check the result of Frame.unwind_stop_reason,
> and compare it to gdb.FRAME_UNWIND_NO_REASON.  That at most, they'll
> pass the result of Frame.unwind_stop_reason to
> gdb.frame_stop_reason_string.  I'd prefer to just get rid of it, but
> it may be best to keep this around for compatibility, in case a script
> does refer to gdb.FRAME_UNWIND_NULL_ID directly.
> 
> In general, what's the policy for exposed constants like this in
> Python?
> 
> gdb/
> 2013-11-28  Pedro Alves  <palves@redhat.com>
> 
> 	* unwind_stop_reasons.def (UNWIND_NULL_ID): Update comment.
> 
> gdb/doc/
> 2013-11-28  Pedro Alves  <palves@redhat.com>
> 
> 	* gdb.texinfo (Frames In Python) <gdb.FRAME_UNWIND_NULL_ID>:
> 	Update comment.
> ---

As a script could check it, and because we make an API promise, we
really have to leave it in there.  You have done the correct
thing by documenting the deprecated nature of the constant.

It might be time to think about what the API pertains too.  GDB
evolves constantly, and I really don't want Python in a few years from
now to be full of deprecated functions/constants.

OK by me.

Cheers,

Phil

Re: [RFC] Python's gdb.FRAME_UNWIND_NULL_ID is no longer used. What to do with it?

[Pedro Alves]

On 11/28/2013 09:16 PM, Phil Muldoon wrote:

> As a script could check it, and because we make an API promise, we
> really have to leave it in there.  You have done the correct
> thing by documenting the deprecated nature of the constant.
> 
> It might be time to think about what the API pertains too.  GDB
> evolves constantly, and I really don't want Python in a few years from
> now to be full of deprecated functions/constants.
> 
> OK by me.

Thanks guys.  I've pushed it.

-- 
Pedro Alves

Re: [RFC] Python's gdb.FRAME_UNWIND_NULL_ID is no longer used. What to do with it?

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> I'd assume scripts just check the result of Frame.unwind_stop_reason,
Pedro> and compare it to gdb.FRAME_UNWIND_NO_REASON.  That at most, they'll
Pedro> pass the result of Frame.unwind_stop_reason to
Pedro> gdb.frame_stop_reason_string.  I'd prefer to just get rid of it, but
Pedro> it may be best to keep this around for compatibility, in case a script
Pedro> does refer to gdb.FRAME_UNWIND_NULL_ID directly.

Pedro> In general, what's the policy for exposed constants like this in
Pedro> Python?

My view is that we should provide compatibility with what we document.

That is, if it is in the docs, we should not remove it.

However, there is still some leeway for change.
E.g., in this case, we don't document the value or type of the constant.
This is intentional as it gives us some freedom to change the details --
we don't generally want Python API promises to leak through to the C
code.

Pedro> +/* This is no longer used anywhere, but it's kept because it's exposed
Pedro> +   to Python.  This is how GDB used to indicate end of stack.  We've
Pedro> +   now migrated to a model where frames always have a valid ID.  */
Pedro>  SET (UNWIND_NULL_ID, "unwinder did not report frame ID")
 
For example you could remove this, and arrange to keep the Python
constant around with some suitably chosen value.  I think it just has to
be something that will never compare equal to any of the other
constants, so just None would work.

Tom

Re: [RFC] Python's gdb.FRAME_UNWIND_NULL_ID is no longer used. What to do with it?

[Pedro Alves]

On 12/02/2013 04:10 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> I'd assume scripts just check the result of Frame.unwind_stop_reason,
> Pedro> and compare it to gdb.FRAME_UNWIND_NO_REASON.  That at most, they'll
> Pedro> pass the result of Frame.unwind_stop_reason to
> Pedro> gdb.frame_stop_reason_string.  I'd prefer to just get rid of it, but
> Pedro> it may be best to keep this around for compatibility, in case a script
> Pedro> does refer to gdb.FRAME_UNWIND_NULL_ID directly.
> 
> Pedro> In general, what's the policy for exposed constants like this in
> Pedro> Python?
> 
> My view is that we should provide compatibility with what we document.
> 
> That is, if it is in the docs, we should not remove it.
> 
> However, there is still some leeway for change.
> E.g., in this case, we don't document the value or type of the constant.
> This is intentional as it gives us some freedom to change the details --
> we don't generally want Python API promises to leak through to the C
> code.
> 
> Pedro> +/* This is no longer used anywhere, but it's kept because it's exposed
> Pedro> +   to Python.  This is how GDB used to indicate end of stack.  We've
> Pedro> +   now migrated to a model where frames always have a valid ID.  */
> Pedro>  SET (UNWIND_NULL_ID, "unwinder did not report frame ID")
>  
> For example you could remove this, and arrange to keep the Python
> constant around with some suitably chosen value.  I think it just has to
> be something that will never compare equal to any of the other
> constants, so just None would work.

I tried that, and then I remembered to try frame_stop_reason_string
on it:

(top-gdb) python print gdb.FRAME_UNWIND_NULL_ID
None
(top-gdb) python print gdb.FRAME_UNWIND_OUTERMOST
1
(top-gdb) python print gdb.frame_stop_reason_string(gdb.FRAME_UNWIND_OUTERMOST)
outermost
(top-gdb) python print gdb.frame_stop_reason_string(gdb.FRAME_UNWIND_NULL_ID)
Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: an integer is required
Error while executing Python code.

GDB itself won't ever generate FRAME_UNWIND_NULL_ID.  Do think
that's a problem? I guess we could set it to UNWIND_LAST+1
instead, and make gdbpy_frame_stop_reason_string handle it,
but at that point, I get to consider just leaving things
alone...

Hmm, or perhaps, we could implement this another way.  Leave
UNWIND_NULL_ID in unwind_stop_reasons.def, but define it
with a DEPRECATED_SET macro instead.  I guess that might
actually be better considering multiple extension languages
going forward, as then we have a single place to do this,
instead of having to repeat the "create deprecated constant"
exercise for all extension languages whenever something like
this happens.

--------------------
gdb.FRAME_UNWIND_NULL_ID is documented, so we need to keep it around.
However, we can just set it to None.

2013-12-12  Pedro Alves  <palves@redhat.com>
	    Tom Tromey  <tromey@redhat.com>

	* python/py-frame.c (gdbpy_initialize_frames): Add
	gdb.FRAME_UNWIND_NULL_ID as a deprecated constant.
	* python/py-utils.c (gdb_pymodule_add_deprecated_constant): New
	function.
	* python/python-internal.h (gdb_pymodule_add_deprecated_constant):
	Declare.
	* unwind_stop_reasons.def (UNWIND_NULL_ID): Delete.
---
 gdb/python/py-frame.c        |  6 ++++++
 gdb/python/py-utils.c        |  8 ++++++++
 gdb/python/python-internal.h | 14 ++++++++++++++
 gdb/unwind_stop_reasons.def  |  5 -----
 4 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 0f189f0..325b81b 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -649,6 +649,12 @@ gdbpy_initialize_frames (void)
 #include "unwind_stop_reasons.def"
 #undef SET

+  /* GDB core doesn't use this anymore, but since it was exposed to
+     Python before and documented, we can't remove it.  */
+  if (gdb_pymodule_add_deprecated_constant (gdb_module,
+					    "FRAME_UNWIND_NULL_ID") < 0)
+    return -1;
+
   return gdb_pymodule_addobject (gdb_module, "Frame",
 				 (PyObject *) &frame_object_type);
 }
diff --git a/gdb/python/py-utils.c b/gdb/python/py-utils.c
index 4dcd168..32340bf 100644
--- a/gdb/python/py-utils.c
+++ b/gdb/python/py-utils.c
@@ -442,3 +442,11 @@ gdb_pymodule_addobject (PyObject *module, const char *name, PyObject *object)
     Py_DECREF (object);
   return result;
 }
+
+/* See python-internal.h.  */
+
+int
+gdb_pymodule_add_deprecated_constant (PyObject *module, const char *name)
+{
+  return gdb_pymodule_addobject (gdb_module, name, Py_None);
+}
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 125670e..2fb0c8d 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -494,4 +494,18 @@ int gdb_pymodule_addobject (PyObject *module, const char *name,
 			    PyObject *object)
   CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;

+/* Adds a deprecated constant to MODULE as NAME.  Return -1 on error,
+   0 on success.  The constant is set to None.
+
+   Rationale: We don't generally want Python API promises to leak
+   through to GDB's core C code.  We intentionaly usually don't
+   document the value or type of Python constants, as it gives us some
+   freedom to change the details.  When a core constant that was
+   exposed to Python is removed from the core, we arrange to keep the
+   Python constant around with a value that never compares equal to
+   any of the non-deprecated constants.  */
+int gdb_pymodule_add_deprecated_constant (PyObject *module,
+					  const char *name)
+  CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
+
 #endif /* GDB_PYTHON_INTERNAL_H */
diff --git a/gdb/unwind_stop_reasons.def b/gdb/unwind_stop_reasons.def
index 2c3d341..da67f21 100644
--- a/gdb/unwind_stop_reasons.def
+++ b/gdb/unwind_stop_reasons.def
@@ -31,11 +31,6 @@
    or we didn't fail.  */
 SET (UNWIND_NO_REASON, "no reason")

-/* This is no longer used anywhere, but it's kept because it's exposed
-   to Python.  This is how GDB used to indicate end of stack.  We've
-   now migrated to a model where frames always have a valid ID.  */
-SET (UNWIND_NULL_ID, "unwinder did not report frame ID")
-
 /* This frame is the outermost.  */
 SET (UNWIND_OUTERMOST, "outermost")

-- 
1.7.11.7

Re: [RFC] Python's gdb.FRAME_UNWIND_NULL_ID is no longer used. What to do with it?

[Pedro Alves]

On 12/12/2013 05:25 PM, Pedro Alves wrote:
> On 12/02/2013 04:10 PM, Tom Tromey wrote:
>>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>>
>> Pedro> I'd assume scripts just check the result of Frame.unwind_stop_reason,
>> Pedro> and compare it to gdb.FRAME_UNWIND_NO_REASON.  That at most, they'll
>> Pedro> pass the result of Frame.unwind_stop_reason to
>> Pedro> gdb.frame_stop_reason_string.  I'd prefer to just get rid of it, but
>> Pedro> it may be best to keep this around for compatibility, in case a script
>> Pedro> does refer to gdb.FRAME_UNWIND_NULL_ID directly.
>>
>> Pedro> In general, what's the policy for exposed constants like this in
>> Pedro> Python?
>>
>> My view is that we should provide compatibility with what we document.
>>
>> That is, if it is in the docs, we should not remove it.
>>
>> However, there is still some leeway for change.
>> E.g., in this case, we don't document the value or type of the constant.
>> This is intentional as it gives us some freedom to change the details --
>> we don't generally want Python API promises to leak through to the C
>> code.
>>
>> Pedro> +/* This is no longer used anywhere, but it's kept because it's exposed
>> Pedro> +   to Python.  This is how GDB used to indicate end of stack.  We've
>> Pedro> +   now migrated to a model where frames always have a valid ID.  */
>> Pedro>  SET (UNWIND_NULL_ID, "unwinder did not report frame ID")
>>  
>> For example you could remove this, and arrange to keep the Python
>> constant around with some suitably chosen value.  I think it just has to
>> be something that will never compare equal to any of the other
>> constants, so just None would work.
> 
> I tried that, and then I remembered to try frame_stop_reason_string
> on it:
> 
> (top-gdb) python print gdb.FRAME_UNWIND_NULL_ID
> None
> (top-gdb) python print gdb.FRAME_UNWIND_OUTERMOST
> 1
> (top-gdb) python print gdb.frame_stop_reason_string(gdb.FRAME_UNWIND_OUTERMOST)
> outermost
> (top-gdb) python print gdb.frame_stop_reason_string(gdb.FRAME_UNWIND_NULL_ID)
> Traceback (most recent call last):
>   File "<string>", line 1, in <module>
> TypeError: an integer is required
> Error while executing Python code.
> 
> GDB itself won't ever generate FRAME_UNWIND_NULL_ID.  Do think
> that's a problem? I guess we could set it to UNWIND_LAST+1
> instead, and make gdbpy_frame_stop_reason_string handle it,
> but at that point, I get to consider just leaving things
> alone...
> 
> Hmm, or perhaps, we could implement this another way.  Leave
> UNWIND_NULL_ID in unwind_stop_reasons.def, but define it
> with a DEPRECATED_SET macro instead.  I guess that might
> actually be better considering multiple extension languages
> going forward, as then we have a single place to do this,
> instead of having to repeat the "create deprecated constant"
> exercise for all extension languages whenever something like
> this happens.

Well, I tried that, but the result doesn't look that
much better to me.  I'm not really motivated to handle
this right now, so I'll just forget about it for now.
(others do please feel free to carry on.  I do believe
we'll end up needing to set some precedent.)

-- 
Pedro Alves