diff mbox series

[11/14] Remove redundant check from tui_refresh_frame_and_register_information

Message ID 20231217-tui-regs-cleanup-v1-11-67bd0ea1e8be@tromey.com
State New
Headers
Series Cleanups for the TUi register window |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Tom Tromey Dec. 17, 2023, 7:50 p.m. UTC 
  tui_refresh_frame_and_register_information checks 'from_stack' in a
block that's already guarded by a 'from_stack' check.  This patch
removes the redundant check.
---
 gdb/tui/tui-hooks.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Hannes Domani Dec. 18, 2023, 5:32 p.m. UTC | #1 
Am Sonntag, 17. Dezember 2023, 20:51:57 MEZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> tui_refresh_frame_and_register_information checks 'from_stack' in a
> block that's already guarded by a 'from_stack' check.  This patch
> removes the redundant check.
> ---
> gdb/tui/tui-hooks.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
> index e47607fefa9..f1e4978a40a 100644
> --- a/gdb/tui/tui-hooks.c
> +++ b/gdb/tui/tui-hooks.c
> @@ -132,11 +132,10 @@ tui_refresh_frame_and_register_information ()
>
>       /* Display the frame position (even if there is no symbols or
>     the PC is not known).  */
> -      bool frame_info_changed_p = tui_show_frame_info (fi);
> +      tui_show_frame_info (fi);
>
>       /* Refresh the register window if it's visible.  */
> -      if (tui_is_window_visible (DATA_WIN)
> -      && (frame_info_changed_p || from_stack))
> +      if (tui_is_window_visible (DATA_WIN))
>      TUI_DATA_WIN->check_register_values (fi);
>     }
>   else if (!from_stack)
>
> --
> 2.43.0

Was the return value of tui_show_frame_info(fi) also useless?


Regards
Hannes
Tom Tromey Dec. 18, 2023, 11:35 p.m. UTC | #2 
>>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:

>> -      if (tui_is_window_visible (DATA_WIN)
>> -      && (frame_info_changed_p || from_stack))

Hannes> Was the return value of tui_show_frame_info(fi) also useless?

Yes, in the above, 'from_stack' is always true, so the value of
'frame_info_changed_p' is immaterial.

Tom
Andrew Burgess Dec. 19, 2023, 10:26 a.m. UTC | #3 
Tom Tromey <tom@tromey.com> writes:

>>>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:
>
>>> -      if (tui_is_window_visible (DATA_WIN)
>>> -      && (frame_info_changed_p || from_stack))
>
> Hannes> Was the return value of tui_show_frame_info(fi) also useless?
>
> Yes, in the above, 'from_stack' is always true, so the value of
> 'frame_info_changed_p' is immaterial.

Except I think that's a bug in the original code.

As your previous patch points out we already hook the register changed
observer, so the only time that a register change will need this
function to refresh the register display is if the frame was changed.

If we're going to stop checking frame_info_changed_p then we might as
well stop hooking gdb::observers::register_changed and just rely on the
before prompt hook to update everything, right?

Thanks,
Andrew
Tom Tromey Jan. 20, 2024, 6:21 p.m. UTC | #4 
>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

>> Yes, in the above, 'from_stack' is always true, so the value of
>> 'frame_info_changed_p' is immaterial.

Andrew> Except I think that's a bug in the original code.

Andrew> As your previous patch points out we already hook the register changed
Andrew> observer, so the only time that a register change will need this
Andrew> function to refresh the register display is if the frame was changed.

Andrew> If we're going to stop checking frame_info_changed_p then we might as
Andrew> well stop hooking gdb::observers::register_changed and just rely on the
Andrew> before prompt hook to update everything, right?

I don't think that will work correctly, because the TUI register window
shows changed registers in a special way.  If we rely solely on the
before-prompt hook, then any command at all will cause a redisplay that
will erase the "changed" highlighting.

This is why two phases are needed: the register window hooks into the
various things that can cause a register change, and then this is
checked before redisplay.

The patch in question here just removes some code that checks if the
frame changed.  However, if the frame did change, from_stack is already
set due to the context-changed observer.

Tom
diff mbox series

Patch

diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
index e47607fefa9..f1e4978a40a 100644
--- a/gdb/tui/tui-hooks.c
+++ b/gdb/tui/tui-hooks.c
@@ -132,11 +132,10 @@  tui_refresh_frame_and_register_information ()
 
       /* Display the frame position (even if there is no symbols or
 	 the PC is not known).  */
-      bool frame_info_changed_p = tui_show_frame_info (fi);
+      tui_show_frame_info (fi);
 
       /* Refresh the register window if it's visible.  */
-      if (tui_is_window_visible (DATA_WIN)
-	  && (frame_info_changed_p || from_stack))
+      if (tui_is_window_visible (DATA_WIN))
 	TUI_DATA_WIN->check_register_values (fi);
     }
   else if (!from_stack)