[v3,3/4] Deprecate frame_stop_reason_string.
Commit Message
On 28/05/2014 6:26 PM, Pedro Alves wrote:
> On 04/30/2014 11:55 AM, Andrew Burgess wrote:
>> Patch #4 adds frame specific stop reason strings. It is better to use that
>> string rather than the existing generic stop reason string. This patch
>> renames frame_stop_reason_string to deprecated_frame_stop_reason_string and
>> updates all the call sites to use the deprecated name.
>>
>> Patch #4 adds the new frame specific stop reason, and some uses of the
>> deprecated funciton will be moved to this new API, however, in the python
>> and guile scripting API we expose a function that converts the stop reason
>> code into a string, without a frame, this will be harder to convert to the
>
> The function still serves it's purpose of mapping the enum values
> to string equivalents. I don't really see a need to mark it deprecated.
I guess not. In my head I figured if you were going to show a string then
you'd always be better going through the new frame specific interface, as
any string it returns might be "more specific" to this frame, and strings
are always (that's my assumption / guess) just being displayed to the user
so more specific would be better.
For program control you'd still have the get_frame_unwind_stop_reason which
returns the enum value.
I wasn't sure where the old function would still serve a really useful
role, and I worried it would be used in places the frame specific version
would be better.
That said, I don't object to keeping it around. It can always be removed
later if it turns out that it's not used.
> IMO, the real problem with its signature is the "frame_" in its name.
> The function converts an 'enum unwind_stop_reason' value to a string,
> so like other similar cases in the tree, how about renaming the function
> to match? E.g.:
>
> -const char *frame_stop_reason_string (enum unwind_stop_reason);
> +const char *unwind_stop_reason_to_string (enum unwind_stop_reason);
I've gone with this suggestion, updated patch below. Is this OK to apply?
Thanks,
Andrew
gdb/ChangeLog:
* frame.c (frame_stop_reason_string): Rename to ...
(unwind_frame_stop_reason_string): this.
* frame.h (frame_stop_reason_string): Rename to ...
(unwind_frame_stop_reason_string): this.
* stack.c (frame_info): Update call to frame_stop_reason_string.
(backtrace_command_1): Likewise.
* guile/scm-frame.c (gdbscm_unwind_stop_reason_string): Likewise.
* python/py-frame.c (gdbpy_frame_stop_reason_string): Likewise.
---
gdb/frame.c | 2 +-
gdb/frame.h | 6 ++++--
gdb/guile/scm-frame.c | 2 +-
gdb/python/py-frame.c | 2 +-
gdb/stack.c | 4 ++--
5 files changed, 9 insertions(+), 7 deletions(-)
Comments
On 05/29/2014 12:26 AM, Andrew Burgess wrote:
> On 28/05/2014 6:26 PM, Pedro Alves wrote:
>> On 04/30/2014 11:55 AM, Andrew Burgess wrote:
>> IMO, the real problem with its signature is the "frame_" in its name.
>> The function converts an 'enum unwind_stop_reason' value to a string,
>> so like other similar cases in the tree, how about renaming the function
>> to match? E.g.:
>>
>> -const char *frame_stop_reason_string (enum unwind_stop_reason);
>> +const char *unwind_stop_reason_to_string (enum unwind_stop_reason);
>
> I've gone with this suggestion, updated patch below. Is this OK to apply?
Not yet, sorry.
> * frame.c (frame_stop_reason_string): Rename to ...
> (unwind_frame_stop_reason_string): this.
This still has "frame" in the name, and misses the "to". Any reason for that?
The specific suggestion had a logic --
convert "enum unwind_stop_reason" to "string" -> "unwind_stop_reason_to_string".
That is just like target_waitstatus_to_string and
target_xfer_status_to_string, for example.
Without even looking at the function's declaration, I can tell
that is converting the enum value. While "unwind_frame_stop_reason_string"
without the "to" doesn't give me that impression, and is very much
confusable with the new frame_stop_reason_string.
> * frame.h (frame_stop_reason_string): Rename to ...
> (unwind_frame_stop_reason_string): this.
> * stack.c (frame_info): Update call to frame_stop_reason_string.
> (backtrace_command_1): Likewise.
> * guile/scm-frame.c (gdbscm_unwind_stop_reason_string): Likewise.
> * python/py-frame.c (gdbpy_frame_stop_reason_string): Likewise.
...
> diff --git a/gdb/frame.h b/gdb/frame.h
> index ad03a0b..5acb2a2 100644
> --- a/gdb/frame.h
> +++ b/gdb/frame.h
> @@ -501,9 +501,11 @@ enum unwind_stop_reason
>
> enum unwind_stop_reason get_frame_unwind_stop_reason (struct frame_info *);
>
> -/* Translate a reason code to an informative string. */
> +/* Translate a reason code to an informative string. This returns a
> + general string describing the stop reason, for a possibly frame
> + specific reason string, use frame_stop_reason_string. */
Please remove this comment hunk from this patch, so that the patch
remains independent, and so that it can go in immediately even if
further discuss patch #4.
@@ -2561,7 +2561,7 @@ get_frame_unwind_stop_reason (struct frame_info *frame)
/* Return a string explaining REASON. */
const char *
-frame_stop_reason_string (enum unwind_stop_reason reason)
+unwind_frame_stop_reason_string (enum unwind_stop_reason reason)
{
switch (reason)
{
@@ -501,9 +501,11 @@ enum unwind_stop_reason
enum unwind_stop_reason get_frame_unwind_stop_reason (struct frame_info *);
-/* Translate a reason code to an informative string. */
+/* Translate a reason code to an informative string. This returns a
+ general string describing the stop reason, for a possibly frame
+ specific reason string, use frame_stop_reason_string. */
-const char *frame_stop_reason_string (enum unwind_stop_reason);
+const char *unwind_frame_stop_reason_string (enum unwind_stop_reason);
/* Unwind the stack frame so that the value of REGNUM, in the previous
(up, older) frame is returned. If VALUEP is NULL, don't
@@ -933,7 +933,7 @@ gdbscm_unwind_stop_reason_string (SCM reason_scm)
if (reason < UNWIND_FIRST || reason > UNWIND_LAST)
scm_out_of_range (FUNC_NAME, reason_scm);
- str = frame_stop_reason_string (reason);
+ str = unwind_frame_stop_reason_string (reason);
return gdbscm_scm_from_c_string (str);
}
@@ -588,7 +588,7 @@ gdbpy_frame_stop_reason_string (PyObject *self, PyObject *args)
return NULL;
}
- str = frame_stop_reason_string (reason);
+ str = unwind_frame_stop_reason_string (reason);
return PyUnicode_Decode (str, strlen (str), host_charset (), NULL);
}
@@ -1529,7 +1529,7 @@ frame_info (char *addr_exp, int from_tty)
reason = get_frame_unwind_stop_reason (fi);
if (reason != UNWIND_NO_REASON)
printf_filtered (_(" Outermost frame: %s\n"),
- frame_stop_reason_string (reason));
+ unwind_frame_stop_reason_string (reason));
}
else if (get_frame_type (fi) == TAILCALL_FRAME)
puts_filtered (" tail call frame");
@@ -1848,7 +1848,7 @@ backtrace_command_1 (char *count_exp, int show_locals, int no_filters,
reason = get_frame_unwind_stop_reason (trailing);
if (reason >= UNWIND_FIRST_ERROR)
printf_filtered (_("Backtrace stopped: %s\n"),
- frame_stop_reason_string (reason));
+ unwind_frame_stop_reason_string (reason));
}
}
}