[3/3] Replace TUI's select_frame hook (PR tui/13378)
Commit Message
Actually, we should not call tui_show_frame_info(NULL) when there are no
stack frames, because doing so empties the source window after invoking
"continue&". That was a last-minute change with no justification anyway.
Here is a revised patch with a much smaller diffstat.
gdb/ChangeLog:
* frame.c (select_frame): Remove reference to
deprecated_selected_frame_level_changed_hook.
* frame.h (deprecated_selected_frame_level_changed_hook): Remove
declaration.
* stack.c (deprecated_selected_frame_level_changed_hook):
Likewise.
* tui/tui-hooks.c (tui_selected_frame_level_changed_hook):
Rename to ...
(tui_refresh_frame_and_register_information): ... this. Bail
out if there is no stack.
(tui_before_prompt): New function.
(tui_before_prompt_observer): New observer.
(tui_install_hooks): Register tui_before_prompt_observer to
call tui_before_prompt. Remove reference to
deprecated_selected_frame_level_changed_hook.
(tui_remove_hooks): Detach and unset tui_before_prompt_observer.
Remove reference to
deprecated_selected_frame_level_changed_hook.
* tui/tui-hooks.h (tui_refresh_frame_and_register_information):
Export.
* tui/tui-interp.c: Include "tui/tui-hooks.h".
(tui_on_sync_execution_done): Call
tui_refresh_frame_and_register_information if tui_active.
---
gdb/frame.c | 2 --
gdb/frame.h | 2 --
gdb/stack.c | 2 --
gdb/tui/tui-hooks.c | 28 ++++++++++++++++++----------
gdb/tui/tui-hooks.h | 5 +++++
gdb/tui/tui-interp.c | 4 ++++
6 files changed, 27 insertions(+), 16 deletions(-)
Comments
On 06/27/2015 03:50 AM, Patrick Palka wrote:
> Actually, we should not call tui_show_frame_info(NULL) when there are no
> stack frames, because doing so empties the source window after invoking
> "continue&". That was a last-minute change with no justification anyway.
>
> Here is a revised patch with a much smaller diffstat.
On 06/27/2015 03:35 AM, Patrick Palka wrote:
> Effectively, with this patch, frame/PC changes that do not immediately
> precede an inferior-stop event or a prompt display event no longer cause
> TUI's frame and register information to be updated.
It seems to me that this doesn't update the info when the
program stops after continue& ...
> @@ -107,6 +108,9 @@ tui_on_sync_execution_done (void)
> {
> if (!interp_quiet_p (tui_interp))
> display_gdb_prompt (NULL);
> +
> + if (tui_active)
> + tui_refresh_frame_and_register_information ();
> }
>
... when sync/foreground execution is done, we show the
prompt (display_gdb_prompt above), so the before_prompt observer
is already taking care of refreshing.
But when background/async execution is done, we don't call
sync_execution_done observers, so this hook isn't called.
So I'm not seeing what this bit in tui_on_sync_execution_done
is for.
I think that for for stops (either after background or foreground
execution), the TUI refreshes itself from within the
deprecated_print_frame_info_listing_hook, called from:
#0 tui_show_frame_info (fi=0xf20b50) at /home/pedro/gdb/mygit/build/../src/gdb/tui/tui-stack.c:337
#1 0x00000000005266c5 in tui_print_frame_info_listing_hook (s=0x1978100, line=92, stopline=93, noerror=0) at /home/pedro/gdb/mygit/build/../src/gdb/tui/tui-hooks.c:177
#2 0x000000000063a6f7 in print_frame_info (frame=0xf20b50, print_level=0, print_what=SRC_AND_LOC, print_args=1, set_current_sal=1)
at /home/pedro/gdb/mygit/build/../src/gdb/stack.c:895
#3 0x0000000000638e2f in print_stack_frame (frame=0xf20b50, print_level=0, print_what=SRC_AND_LOC, set_current_sal=1)
at /home/pedro/gdb/mygit/build/../src/gdb/stack.c:162
#4 0x0000000000632c14 in print_stop_event (ws=0x7fffffffd120) at /home/pedro/gdb/mygit/build/../src/gdb/infrun.c:6536
#5 0x0000000000632fa5 in normal_stop () at /home/pedro/gdb/mygit/build/../src/gdb/infrun.c:6701
I think this hooking in deprecated_print_frame_info_listing_hook
is also a too-low-level place, and is likely contributing
to flicker in other situations. I think we should move
to doing whatever we do in it to a normal_stop observer instead.
(tbc, doesn't have to be this patch.)
> And as a result of this change and of the previous change to
> tui_show_frame_info, the TUI is much more disciplined about updating the
> screen, and so the flicker as described in the PR is totally gone.
I think we should sort out patch 3 before patch 2. AFAICS, the
important one is this one, right?
Thanks,
Pedro Alves
On Tue, Jun 30, 2015 at 5:32 AM, Pedro Alves <palves@redhat.com> wrote:
> On 06/27/2015 03:50 AM, Patrick Palka wrote:
>> Actually, we should not call tui_show_frame_info(NULL) when there are no
>> stack frames, because doing so empties the source window after invoking
>> "continue&". That was a last-minute change with no justification anyway.
>>
>> Here is a revised patch with a much smaller diffstat.
>
> On 06/27/2015 03:35 AM, Patrick Palka wrote:
>
>> Effectively, with this patch, frame/PC changes that do not immediately
>> precede an inferior-stop event or a prompt display event no longer cause
>> TUI's frame and register information to be updated.
>
> It seems to me that this doesn't update the info when the
> program stops after continue& ...
Oops, yeah..
>
>> @@ -107,6 +108,9 @@ tui_on_sync_execution_done (void)
>> {
>> if (!interp_quiet_p (tui_interp))
>> display_gdb_prompt (NULL);
>> +
>> + if (tui_active)
>> + tui_refresh_frame_and_register_information ();
>> }
>>
>
> ... when sync/foreground execution is done, we show the
> prompt (display_gdb_prompt above), so the before_prompt observer
> is already taking care of refreshing.
>
> But when background/async execution is done, we don't call
> sync_execution_done observers, so this hook isn't called.
> So I'm not seeing what this bit in tui_on_sync_execution_done
> is for.
Good point... the sync_execution_done addendum is useless.
>
> I think that for for stops (either after background or foreground
> execution), the TUI refreshes itself from within the
> deprecated_print_frame_info_listing_hook, called from:
>
> #0 tui_show_frame_info (fi=0xf20b50) at /home/pedro/gdb/mygit/build/../src/gdb/tui/tui-stack.c:337
> #1 0x00000000005266c5 in tui_print_frame_info_listing_hook (s=0x1978100, line=92, stopline=93, noerror=0) at /home/pedro/gdb/mygit/build/../src/gdb/tui/tui-hooks.c:177
> #2 0x000000000063a6f7 in print_frame_info (frame=0xf20b50, print_level=0, print_what=SRC_AND_LOC, print_args=1, set_current_sal=1)
> at /home/pedro/gdb/mygit/build/../src/gdb/stack.c:895
> #3 0x0000000000638e2f in print_stack_frame (frame=0xf20b50, print_level=0, print_what=SRC_AND_LOC, set_current_sal=1)
> at /home/pedro/gdb/mygit/build/../src/gdb/stack.c:162
> #4 0x0000000000632c14 in print_stop_event (ws=0x7fffffffd120) at /home/pedro/gdb/mygit/build/../src/gdb/infrun.c:6536
> #5 0x0000000000632fa5 in normal_stop () at /home/pedro/gdb/mygit/build/../src/gdb/infrun.c:6701
>
> I think this hooking in deprecated_print_frame_info_listing_hook
> is also a too-low-level place, and is likely contributing
> to flicker in other situations. I think we should move
> to doing whatever we do in it to a normal_stop observer instead.
> (tbc, doesn't have to be this patch.)
That make sense.
>
>> And as a result of this change and of the previous change to
>> tui_show_frame_info, the TUI is much more disciplined about updating the
>> screen, and so the flicker as described in the PR is totally gone.
>
> I think we should sort out patch 3 before patch 2. AFAICS, the
> important one is this one, right?
This one is more important, but by itself it causes source/asm-window
scrolling to be reset after each command (due to the before_prompt
observer) even if the frame information hasn't been changed in the
meantime. Other than that they are mostly unrelated.
I'll post a revised patch in a bit.
>
> Thanks,
> Pedro Alves
>
On 06/30/2015 01:16 PM, Patrick Palka wrote:
>>> And as a result of this change and of the previous change to
>>> tui_show_frame_info, the TUI is much more disciplined about updating the
>>> screen, and so the flicker as described in the PR is totally gone.
>>
>> I think we should sort out patch 3 before patch 2. AFAICS, the
>> important one is this one, right?
>
> This one is more important, but by itself it causes source/asm-window
> scrolling to be reset after each command (due to the before_prompt
> observer) even if the frame information hasn't been changed in the
> meantime. Other than that they are mostly unrelated.
OK, I see.
I think I missed that because it only happens if you scroll enough to
make the highlighted source line _not_ visible. Just scrolling a bit
and entering a command is not enough to trigger it.
Thanks,
Pedro Alves
On Tue, Jun 30, 2015 at 9:23 AM, Pedro Alves <palves@redhat.com> wrote:
> On 06/30/2015 01:16 PM, Patrick Palka wrote:
>
>>>> And as a result of this change and of the previous change to
>>>> tui_show_frame_info, the TUI is much more disciplined about updating the
>>>> screen, and so the flicker as described in the PR is totally gone.
>>>
>>> I think we should sort out patch 3 before patch 2. AFAICS, the
>>> important one is this one, right?
>>
>> This one is more important, but by itself it causes source/asm-window
>> scrolling to be reset after each command (due to the before_prompt
>> observer) even if the frame information hasn't been changed in the
>> meantime. Other than that they are mostly unrelated.
>
> OK, I see.
>
> I think I missed that because it only happens if you scroll enough to
> make the highlighted source line _not_ visible. Just scrolling a bit
> and entering a command is not enough to trigger it.
Yep -- sorry, should've mentioned that bit.
>
> Thanks,
> Pedro Alves
>
@@ -1576,8 +1576,6 @@ select_frame (struct frame_info *fi)
selected_frame = fi;
/* NOTE: cagney/2002-05-04: FI can be NULL. This occurs when the
frame is being invalidated. */
- if (deprecated_selected_frame_level_changed_hook)
- deprecated_selected_frame_level_changed_hook (frame_relative_level (fi));
/* FIXME: kseitz/2002-08-28: It would be nice to call
selected_frame_level_changed_event() right here, but due to limitations
@@ -763,8 +763,6 @@ extern void args_info (char *, int);
extern void locals_info (char *, int);
-extern void (*deprecated_selected_frame_level_changed_hook) (int);
-
extern void return_command (char *, int);
/* Set FRAME's unwinder temporarily, so that we can call a sniffer.
@@ -52,8 +52,6 @@
#include "symfile.h"
#include "extension.h"
-void (*deprecated_selected_frame_level_changed_hook) (int);
-
/* The possible choices of "set print frame-arguments", and the value
of this setting. */
@@ -119,18 +119,16 @@ tui_about_to_proceed (void)
tui_target_has_run = 1;
}
-/* The selected frame has changed. This is happens after a target
- stop or when the user explicitly changes the frame
- (up/down/thread/...). */
-static void
-tui_selected_frame_level_changed_hook (int level)
+/* See tui-hooks.h. */
+
+void
+tui_refresh_frame_and_register_information (void)
{
struct frame_info *fi;
CORE_ADDR pc;
struct cleanup *old_chain;
- /* Negative level means that the selected frame was cleared. */
- if (level < 0)
+ if (!has_stack_frames ())
return;
old_chain = make_cleanup_restore_target_terminal ();
@@ -191,19 +189,26 @@ tui_inferior_exit (struct inferior *inf)
tui_display_main ();
}
+/* Observer for the before_prompt notification. */
+
+static void
+tui_before_prompt (const char *current_gdb_prompt)
+{
+ tui_refresh_frame_and_register_information ();
+}
+
/* Observers created when installing TUI hooks. */
static struct observer *tui_bp_created_observer;
static struct observer *tui_bp_deleted_observer;
static struct observer *tui_bp_modified_observer;
static struct observer *tui_inferior_exit_observer;
static struct observer *tui_about_to_proceed_observer;
+static struct observer *tui_before_prompt_observer;
/* Install the TUI specific hooks. */
void
tui_install_hooks (void)
{
- deprecated_selected_frame_level_changed_hook
- = tui_selected_frame_level_changed_hook;
deprecated_print_frame_info_listing_hook
= tui_print_frame_info_listing_hook;
@@ -218,6 +223,8 @@ tui_install_hooks (void)
= observer_attach_inferior_exit (tui_inferior_exit);
tui_about_to_proceed_observer
= observer_attach_about_to_proceed (tui_about_to_proceed);
+ tui_before_prompt_observer
+ = observer_attach_before_prompt (tui_before_prompt);
deprecated_register_changed_hook = tui_register_changed_hook;
}
@@ -226,7 +233,6 @@ tui_install_hooks (void)
void
tui_remove_hooks (void)
{
- deprecated_selected_frame_level_changed_hook = 0;
deprecated_print_frame_info_listing_hook = 0;
deprecated_query_hook = 0;
deprecated_register_changed_hook = 0;
@@ -242,6 +248,8 @@ tui_remove_hooks (void)
tui_inferior_exit_observer = NULL;
observer_detach_about_to_proceed (tui_about_to_proceed_observer);
tui_about_to_proceed_observer = NULL;
+ observer_detach_before_prompt (tui_before_prompt_observer);
+ tui_before_prompt_observer = NULL;
}
void _initialize_tui_hooks (void);
@@ -20,6 +20,11 @@
#ifndef TUI_HOOKS_H
#define TUI_HOOKS_H
+/* Refresh TUI's frame and register information. This is a hook intended to be
+ used to update the screen after potential frame and register changes. */
+
+extern void tui_refresh_frame_and_register_information (void);
+
extern void tui_install_hooks (void);
extern void tui_remove_hooks (void);
@@ -27,6 +27,7 @@
#include "tui/tui-data.h"
#include "readline/readline.h"
#include "tui/tui-win.h"
+#include "tui/tui-hooks.h"
#include "tui/tui.h"
#include "tui/tui-io.h"
#include "infrun.h"
@@ -107,6 +108,9 @@ tui_on_sync_execution_done (void)
{
if (!interp_quiet_p (tui_interp))
display_gdb_prompt (NULL);
+
+ if (tui_active)
+ tui_refresh_frame_and_register_information ();
}
/* Observer for the command_error notification. */