diff mbox

Make "list" work again in TUI

Message ID 87in36qu4t.fsf@tromey.com
State New
Headers show

Commit Message

Tom Tromey Sept. 15, 2018, 10:21 p.m. UTC
Pedro> That might suggest instead to start by seeing about trying to remove
Pedro> the select_source_symtab call from tui_refresh_frame_and_register_information.
Pedro> Why do we need that?  Can we remove it and instead see about making
Pedro> tui_refresh_frame_and_register_information update the source window
Pedro> from get_current_source_symtab_and_line, if set_current_source_symtab_and_line
Pedro> was meanwhile called?  I.e., refresh the source window if something changed
Pedro> "list"'s current source&line.

Tom> I'm going to give it a try.

I can almost get everything working by tracking both changes to the
source SAL and changes to the "user select context", and then gating
what happens in tui_refresh_frame_and_register_information based on
which thing changed last.

However this code in tui-winsource.c:a

	  sal.line = line_or_addr.u.line_no +
	    (win_info->generic.content_size - 2);
	  sal.symtab = s;
	  sal.pspace = SYMTAB_PSPACE (s);
	  set_current_source_symtab_and_line (sal);

triggers this warning in source.c:

		  printf_unfiltered ("Line number %d out of range; "
				     "%s has %d lines.\n",
				     line_no,
				     symtab_to_filename_for_display (s),
				     s->nlines);

I don't really understand this.

Simply tracking the SAL is not enough, I think, because you want the TUI
to react to changes that do not have a SAL.  For example, up/down when
the frames do not have debug info.

Aside from the warning the other bug I see now is that if I "layout
src", stop in a frame without debuginfo and then "layout asm", the view
won't show the assembly until I up+down.

Also during this adventure I found out about the "update" command which
just does this:

  execute_command ("frame 0", from_tty);

So this is intended to work at least.


I may keep plugging away at this, who knows.  The TUI really needs work.

I'm appending my WIP patch.

Tom
diff mbox

Patch

diff --git a/gdb/observable.c b/gdb/observable.c
index 5539b9837b5..cb967993cce 100644
--- a/gdb/observable.c
+++ b/gdb/observable.c
@@ -74,6 +74,7 @@  DEFINE_OBSERVABLE (inferior_call_pre);
 DEFINE_OBSERVABLE (inferior_call_post);
 DEFINE_OBSERVABLE (register_changed);
 DEFINE_OBSERVABLE (user_selected_context_changed);
+DEFINE_OBSERVABLE (current_source_symtab_and_line_changed);
 
 } /* namespace observers */
 } /* namespace gdb */
diff --git a/gdb/observable.h b/gdb/observable.h
index 34447b90bb6..8d96cb5dda0 100644
--- a/gdb/observable.h
+++ b/gdb/observable.h
@@ -228,6 +228,12 @@  extern observable<struct frame_info *, int> register_changed;
    frame has changed.  */
 extern observable<user_selected_what> user_selected_context_changed;
 
+/* The CLI's notion of the current source has changed.  This differs
+   from user_selected_context_changed in that it is also set by the
+   "list" command.  */
+
+extern observable<> current_source_symtab_and_line_changed;
+
 } /* namespace observers */
 
 } /* namespace gdb */
diff --git a/gdb/source.c b/gdb/source.c
index ec0ea3b81e3..88c793a752e 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -45,6 +45,7 @@ 
 #include "common/scoped_fd.h"
 #include <algorithm>
 #include "common/pathstuff.h"
+#include "observable.h"
 
 #define OPEN_MODE (O_RDONLY | O_BINARY)
 #define FDOPEN_MODE FOPEN_RB
@@ -201,6 +202,26 @@  set_default_source_symtab_and_line (void)
     select_source_symtab (0);
 }
 
+/* Helper function to set the current source SAL and then notify any
+   observers.  */
+
+static void
+internal_set_current_sal (struct program_space *pspace, struct symtab *symtab,
+			  int line)
+{
+  /* Avoid no-op notifications.  */
+  if (current_source_pspace != pspace
+      || current_source_symtab != symtab
+      || current_source_line != line)
+    {
+      current_source_pspace = pspace;
+      current_source_symtab = symtab;
+      current_source_line = line;
+
+      gdb::observers::current_source_symtab_and_line_changed.notify ();
+    }
+}
+
 /* Return the current default file for listing and next line to list
    (the returned sal pc and end fields are not valid.)
    and set the current default to whatever is in SAL.
@@ -217,9 +238,7 @@  set_current_source_symtab_and_line (const symtab_and_line &sal)
   cursal.pc = 0;
   cursal.end = 0;
 
-  current_source_pspace = sal.pspace;
-  current_source_symtab = sal.symtab;
-  current_source_line = sal.line;
+  internal_set_current_sal (sal.pspace, sal.symtab, sal.line);
 
   /* Force the next "list" to center around the current line.  */
   clear_lines_listed_range ();
@@ -232,8 +251,7 @@  set_current_source_symtab_and_line (const symtab_and_line &sal)
 void
 clear_current_source_symtab_and_line (void)
 {
-  current_source_symtab = 0;
-  current_source_line = 0;
+  internal_set_current_sal (nullptr, nullptr, 0);
 }
 
 /* Set the source file default for the "list" command to be S.
@@ -252,9 +270,7 @@  select_source_symtab (struct symtab *s)
 
   if (s)
     {
-      current_source_symtab = s;
-      current_source_line = 1;
-      current_source_pspace = SYMTAB_PSPACE (s);
+      internal_set_current_sal (SYMTAB_PSPACE (s), s, 1);
       return;
     }
 
@@ -269,9 +285,8 @@  select_source_symtab (struct symtab *s)
 	= decode_line_with_current_source (main_name (),
 					   DECODE_LINE_FUNFIRSTLINE);
       const symtab_and_line &sal = sals[0];
-      current_source_pspace = sal.pspace;
-      current_source_symtab = sal.symtab;
-      current_source_line = std::max (sal.line - (lines_to_list - 1), 1);
+      internal_set_current_sal (sal.pspace, sal.symtab,
+				std::max (sal.line - (lines_to_list - 1), 1));
       if (current_source_symtab)
 	return;
     }
@@ -279,7 +294,7 @@  select_source_symtab (struct symtab *s)
   /* Alright; find the last file in the symtab list (ignoring .h's
      and namespace symtabs).  */
 
-  current_source_line = 1;
+  struct symtab *symtab = current_source_symtab;
 
   ALL_FILETABS (ofp, cu, s)
     {
@@ -288,12 +303,11 @@  select_source_symtab (struct symtab *s)
 
       if (!(len > 2 && (strcmp (&name[len - 2], ".h") == 0
 			|| strcmp (name, "<<C++-namespaces>>") == 0)))
-	{
-	  current_source_pspace = current_program_space;
-	  current_source_symtab = s;
-	}
+	symtab = s;
     }
 
+  internal_set_current_sal (current_program_space, symtab, 1);
+
   if (current_source_symtab)
     return;
 
@@ -302,7 +316,7 @@  select_source_symtab (struct symtab *s)
     if (ofp->sf)
       s = ofp->sf->qf->find_last_source_symtab (ofp);
     if (s)
-      current_source_symtab = s;
+      internal_set_current_sal (current_program_space, s, 1);
   }
   if (current_source_symtab)
     return;
@@ -1255,8 +1269,7 @@  identify_source_line (struct symtab *s, int line, int mid_statement,
   annotate_source (s->fullname, line, s->line_charpos[line - 1],
 		   mid_statement, get_objfile_arch (SYMTAB_OBJFILE (s)), pc);
 
-  current_source_line = line;
-  current_source_symtab = s;
+  internal_set_current_sal (current_source_pspace, s, line);
   clear_lines_listed_range ();
   return 1;
 }
@@ -1276,8 +1289,7 @@  print_source_lines_base (struct symtab *s, int line, int stopline,
   struct ui_out *uiout = current_uiout;
 
   /* Regardless of whether we can open the file, set current_source_symtab.  */
-  current_source_symtab = s;
-  current_source_line = line;
+  internal_set_current_sal (current_source_pspace, s, line);
   first_line_listed = line;
 
   /* If printing of source lines is disabled, just print file and line
@@ -1606,7 +1618,9 @@  forward_search_command (const char *regex, int from_tty)
 	  /* Match!  */
 	  print_source_lines (current_source_symtab, line, line + 1, 0);
 	  set_internalvar_integer (lookup_internalvar ("_"), line);
-	  current_source_line = std::max (line - lines_to_list / 2, 1);
+	  internal_set_current_sal (current_source_pspace,
+				    current_source_symtab,
+				    std::max (line - lines_to_list / 2, 1));
 	  return;
 	}
       line++;
@@ -1677,7 +1691,8 @@  reverse_search_command (const char *regex, int from_tty)
 	  /* Match!  */
 	  print_source_lines (current_source_symtab, line, line + 1, 0);
 	  set_internalvar_integer (lookup_internalvar ("_"), line);
-	  current_source_line = std::max (line - lines_to_list / 2, 1);
+	  internal_set_current_sal (current_source_pspace, current_source_symtab,
+				    std::max (line - lines_to_list / 2, 1));
 	  return;
 	}
       line--;
@@ -1911,7 +1926,6 @@  _initialize_source (void)
 {
   struct cmd_list_element *c;
 
-  current_source_symtab = 0;
   init_source_path ();
 
   /* The intention is to use POSIX Basic Regular Expressions.
diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
index efa02e2f08a..7a55735d53a 100644
--- a/gdb/tui/tui-hooks.c
+++ b/gdb/tui/tui-hooks.c
@@ -48,6 +48,7 @@ 
 #include "tui/tui-winsource.h"
 
 #include "gdb_curses.h"
+#include "source.h"
 
 /* This redefines CTRL if it is not already defined, so it must come
    after terminal state releated include files like <term.h> and
@@ -107,6 +108,23 @@  tui_event_modify_breakpoint (struct breakpoint *b)
   tui_update_all_breakpoint_info ();
 }
 
+static bool context_changed;
+static bool sal_changed;
+
+static void
+tui_sal_changed ()
+{
+  context_changed = false;
+  sal_changed = true;
+}
+
+static void
+tui_context_changed (user_selected_what)
+{
+  context_changed = true;
+  sal_changed = false;
+}
+
 /* 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.
 
@@ -126,6 +144,18 @@  tui_refresh_frame_and_register_information (int registers_too_p)
   target_terminal::scoped_restore_terminal_state term_state;
   target_terminal::ours_for_output ();
 
+  if (sal_changed)
+    {
+      sal_changed = false;
+      struct symtab_and_line sal = get_current_source_symtab_and_line ();
+      tui_update_source_windows_with_line (sal.symtab, sal.line);
+      return;
+    }
+
+  if (!context_changed)
+    return;
+  context_changed = false;
+
   fi = get_selected_frame (NULL);
   /* Ensure that symbols for this frame are read in.  Also, determine
      the source language of this frame, and switch to it if
@@ -271,4 +301,8 @@  _initialize_tui_hooks (void)
 {
   /* Install the permanent hooks.  */
   gdb::observers::new_objfile.attach (tui_new_objfile_hook);
+  gdb::observers::current_source_symtab_and_line_changed.attach
+    (tui_sal_changed);
+  gdb::observers::user_selected_context_changed.attach
+    (tui_context_changed);
 }