From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8513 invoked by alias); 12 Dec 2013 17:25:12 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 8503 invoked by uid 89); 12 Dec 2013 17:25:12 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 12 Dec 2013 17:25:11 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rBCHP9AZ002454 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 12 Dec 2013 12:25:10 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id rBCHP8ff025517; Thu, 12 Dec 2013 12:25:08 -0500 Message-ID: <52A9F173.2010304@redhat.com> Date: Thu, 12 Dec 2013 17:25:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Tom Tromey CC: gdb-patches@sourceware.org Subject: Re: [RFC] Python's gdb.FRAME_UNWIND_NULL_ID is no longer used. What to do with it? References: <1385664356-29726-1-git-send-email-palves@redhat.com> <871u1vw7wt.fsf@fleche.redhat.com> In-Reply-To: <871u1vw7wt.fsf@fleche.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-12/txt/msg00506.txt.bz2 On 12/02/2013 04:10 PM, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves 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 "", line 1, in 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 Tom Tromey * 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