public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/3] gdb/tui: change some macros to functions
@ 2024-05-31  2:54 Simon Marchi
  2024-05-31  2:54 ` [PATCH 2/3] gdb: add IWYU export pragams to gdb_curses.h Simon Marchi
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Simon Marchi @ 2024-05-31  2:54 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@efficios.com>

Change the `TUI_*` macros to access known windows to functions.  Define
them in their respective files, because trying to define them in
tui-data.h would end up causing include cycles.

This makes static analysis (detection of unused include files in this
case) more accurate, and I think in general we should avoid hiding
code behind macros if not necessary.

Change-Id: I1e38cee843984c48ab34030b19dac0d726f851af
---
 gdb/tui/tui-command.c   |  2 +-
 gdb/tui/tui-command.h   |  8 ++++++++
 gdb/tui/tui-data.h      | 11 -----------
 gdb/tui/tui-disasm.c    |  8 ++++----
 gdb/tui/tui-disasm.h    |  9 +++++++++
 gdb/tui/tui-hooks.c     |  4 ++--
 gdb/tui/tui-io.c        | 34 +++++++++++++++++-----------------
 gdb/tui/tui-layout.c    | 20 ++++++++++----------
 gdb/tui/tui-layout.h    |  8 ++++----
 gdb/tui/tui-regs.c      |  6 +++---
 gdb/tui/tui-regs.h      |  8 ++++++++
 gdb/tui/tui-source.h    |  8 ++++++++
 gdb/tui/tui-status.c    |  2 +-
 gdb/tui/tui-status.h    |  8 ++++++++
 gdb/tui/tui-win.c       | 22 +++++++++++-----------
 gdb/tui/tui-winsource.c |  4 ++--
 gdb/tui/tui.c           | 12 ++++++------
 17 files changed, 102 insertions(+), 72 deletions(-)

diff --git a/gdb/tui/tui-command.c b/gdb/tui/tui-command.c
index b0ae8f39e0eb..d36c1372fb88 100644
--- a/gdb/tui/tui-command.c
+++ b/gdb/tui/tui-command.c
@@ -61,7 +61,7 @@ tui_cmd_window::resize (int height_, int width_, int origin_x, int origin_y)
 void
 tui_refresh_cmd_win (void)
 {
-  WINDOW *w = TUI_CMD_WIN->handle.get ();
+  WINDOW *w = tui_cmd_win ()->handle.get ();
 
   tui_wrefresh (w);
 }
diff --git a/gdb/tui/tui-command.h b/gdb/tui/tui-command.h
index 2dc579bce30c..90b8de7535b5 100644
--- a/gdb/tui/tui-command.h
+++ b/gdb/tui/tui-command.h
@@ -55,6 +55,14 @@ struct tui_cmd_window
   int start_line = 0;
 };
 
+/* Return the instance of the command windows.  */
+
+inline tui_cmd_window *
+tui_cmd_win ()
+{
+  return dynamic_cast<tui_cmd_window *> (tui_win_list[CMD_WIN]);
+}
+
 /* Refresh the command window.  */
 extern void tui_refresh_cmd_win (void);
 
diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h
index 3d9e934820d3..d1c855434671 100644
--- a/gdb/tui/tui-data.h
+++ b/gdb/tui/tui-data.h
@@ -290,17 +290,6 @@ struct tui_always_visible_window : public virtual tui_win_info
 /* Global Data.  */
 extern struct tui_win_info *tui_win_list[MAX_MAJOR_WINDOWS];
 
-#define TUI_SRC_WIN \
-  (gdb::checked_static_cast<tui_source_window *> (tui_win_list[SRC_WIN]))
-#define TUI_DISASM_WIN \
-  (gdb::checked_static_cast<tui_disasm_window *> (tui_win_list[DISASSEM_WIN]))
-#define TUI_DATA_WIN \
-  (gdb::checked_static_cast<tui_data_window *> (tui_win_list[DATA_WIN]))
-#define TUI_CMD_WIN \
-  (dynamic_cast<tui_cmd_window *> (tui_win_list[CMD_WIN]))
-#define TUI_STATUS_WIN \
-  (dynamic_cast<tui_status_window *> (tui_win_list[STATUS_WIN]))
-
 /* All the windows that are currently instantiated, in layout
    order.  */
 extern std::vector<tui_win_info *> tui_windows;
diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
index 07ca37603b8c..24a5044ed379 100644
--- a/gdb/tui/tui-disasm.c
+++ b/gdb/tui/tui-disasm.c
@@ -433,12 +433,12 @@ tui_get_low_disassembly_address (struct gdbarch *gdbarch,
 
   /* Determine where to start the disassembly so that the pc is about
      in the middle of the viewport.  */
-  if (TUI_DISASM_WIN != NULL)
-    pos = TUI_DISASM_WIN->height;
-  else if (TUI_CMD_WIN == NULL)
+  if (tui_disasm_win () != nullptr)
+    pos = tui_disasm_win ()->height;
+  else if (tui_cmd_win () == nullptr)
     pos = tui_term_height () / 2 - 2;
   else
-    pos = tui_term_height () - TUI_CMD_WIN->height - 2;
+    pos = tui_term_height () - tui_cmd_win ()->height - 2;
   pos = (pos - 2) / 2;
 
   pc = tui_find_disassembly_address (gdbarch, pc, -pos);
diff --git a/gdb/tui/tui-disasm.h b/gdb/tui/tui-disasm.h
index 0aef091069b7..4a7735450ce4 100644
--- a/gdb/tui/tui-disasm.h
+++ b/gdb/tui/tui-disasm.h
@@ -64,6 +64,15 @@ struct tui_disasm_window : public tui_source_window_base
   bool addr_is_displayed (CORE_ADDR addr) const;
 };
 
+/* Return the instance of the disassembly windows.  */
+
+inline tui_disasm_window *
+tui_disasm_win ()
+{
+  return gdb::checked_static_cast<tui_disasm_window *>
+    (tui_win_list[DISASSEM_WIN]);
+}
+
 extern void tui_get_begin_asm_address (struct gdbarch **, CORE_ADDR *);
 
 #endif /* TUI_TUI_DISASM_H */
diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
index 70874e5460a3..9dcf789ccae9 100644
--- a/gdb/tui/tui-hooks.c
+++ b/gdb/tui/tui-hooks.c
@@ -71,7 +71,7 @@ tui_register_changed (const frame_info_ptr &frame, int regno)
      up in the other.  So we always use the selected frame here, and ignore
      FRAME.  */
   fi = get_selected_frame (NULL);
-  TUI_DATA_WIN->check_register_values (fi);
+  tui_data_win ()->check_register_values (fi);
 }
 
 /* Breakpoint creation hook.
@@ -132,7 +132,7 @@ tui_refresh_frame_and_register_information ()
 
       /* Refresh the register window if it's visible.  */
       if (tui_is_window_visible (DATA_WIN))
-	TUI_DATA_WIN->check_register_values (fi);
+	tui_data_win ()->check_register_values (fi);
     }
   else
     {
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 285c781c25d0..461ac663279b 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -164,7 +164,7 @@ do_tui_putc (WINDOW *w, char c)
 static void
 update_cmdwin_start_line ()
 {
-  TUI_CMD_WIN->start_line = getcury (TUI_CMD_WIN->handle.get ());
+  tui_cmd_win ()->start_line = getcury (tui_cmd_win ()->handle.get ());
 }
 
 /* Print a character in the curses command window.  The output is
@@ -174,7 +174,7 @@ update_cmdwin_start_line ()
 static void
 tui_putc (char c)
 {
-  do_tui_putc (TUI_CMD_WIN->handle.get (), c);
+  do_tui_putc (tui_cmd_win ()->handle.get (), c);
   update_cmdwin_start_line ();
 }
 
@@ -457,7 +457,7 @@ void
 tui_puts (const char *string, WINDOW *w)
 {
   if (w == nullptr)
-    w = TUI_CMD_WIN->handle.get ();
+    w = tui_cmd_win ()->handle.get ();
 
   while (true)
     {
@@ -508,7 +508,7 @@ tui_puts (const char *string, WINDOW *w)
       string = next;
     }
 
-  if (TUI_CMD_WIN != nullptr && w == TUI_CMD_WIN->handle.get ())
+  if (tui_cmd_win () != nullptr && w == tui_cmd_win ()->handle.get ())
     update_cmdwin_start_line ();
 }
 
@@ -552,7 +552,7 @@ tui_puts_internal (WINDOW *w, const char *string, int *height)
 	}
     }
 
-  if (TUI_CMD_WIN != nullptr && w == TUI_CMD_WIN->handle.get ())
+  if (tui_cmd_win () != nullptr && w == tui_cmd_win ()->handle.get ())
     update_cmdwin_start_line ();
   if (saw_nl)
     wrefresh (w);
@@ -582,8 +582,8 @@ tui_redisplay_readline (void)
   
   int c_pos = -1;
   int c_line = -1;
-  WINDOW *w = TUI_CMD_WIN->handle.get ();
-  int start_line = TUI_CMD_WIN->start_line;
+  WINDOW *w = tui_cmd_win ()->handle.get ();
+  int start_line = tui_cmd_win ()->start_line;
   wmove (w, start_line, 0);
   int height = 1;
   if (prompt != nullptr)
@@ -623,17 +623,17 @@ tui_redisplay_readline (void)
 	  waddch (w, c);
 	}
       if (c == '\n')
-	TUI_CMD_WIN->start_line = getcury (w);
+	tui_cmd_win ()->start_line = getcury (w);
       int col = getcurx (w);
       if (col < prev_col)
 	height++;
       prev_col = col;
     }
   wclrtobot (w);
-  TUI_CMD_WIN->start_line = getcury (w);
+  tui_cmd_win ()->start_line = getcury (w);
   if (c_line >= 0)
     wmove (w, c_line, c_pos);
-  TUI_CMD_WIN->start_line -= height - 1;
+  tui_cmd_win ()->start_line -= height - 1;
 
   wrefresh (w);
   fflush(stdout);
@@ -708,7 +708,7 @@ tui_mld_puts (const struct match_list_displayer *displayer, const char *s)
 static void
 tui_mld_flush (const struct match_list_displayer *displayer)
 {
-  wrefresh (TUI_CMD_WIN->handle.get ());
+  wrefresh (tui_cmd_win ()->handle.get ());
 }
 
 /* TUI version of displayer.erase_entire_line.  */
@@ -716,7 +716,7 @@ tui_mld_flush (const struct match_list_displayer *displayer)
 static void
 tui_mld_erase_entire_line (const struct match_list_displayer *displayer)
 {
-  WINDOW *w = TUI_CMD_WIN->handle.get ();
+  WINDOW *w = tui_cmd_win ()->handle.get ();
   int cur_y = getcury (w);
 
   wmove (w, cur_y, 0);
@@ -754,7 +754,7 @@ gdb_wgetch (WINDOW *win)
 static int
 tui_mld_getc (FILE *fp)
 {
-  WINDOW *w = TUI_CMD_WIN->handle.get ();
+  WINDOW *w = tui_cmd_win ()->handle.get ();
   int c = gdb_wgetch (w);
 
   return c;
@@ -1036,7 +1036,7 @@ tui_inject_newline_into_command_window ()
 {
   gdb_assert (tui_active);
 
-  WINDOW *w = TUI_CMD_WIN->handle.get ();
+  WINDOW *w = tui_cmd_win ()->handle.get ();
 
   /* When hitting return with an empty input, gdb executes the last
      command.  If we emit a newline, this fills up the command window
@@ -1061,8 +1061,8 @@ tui_inject_newline_into_command_window ()
       int px, py;
       getyx (w, py, px);
       px += rl_end - rl_point;
-      py += px / TUI_CMD_WIN->width;
-      px %= TUI_CMD_WIN->width;
+      py += px / tui_cmd_win ()->width;
+      px %= tui_cmd_win ()->width;
       wmove (w, py, px);
       tui_putc ('\n');
     }
@@ -1096,7 +1096,7 @@ tui_getc_1 (FILE *fp)
   int ch;
   WINDOW *w;
 
-  w = TUI_CMD_WIN->handle.get ();
+  w = tui_cmd_win ()->handle.get ();
 
 #ifdef TUI_USE_PIPE_FOR_READLINE
   /* Flush readline output.  */
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 143254bcf990..3cb5a05b626a 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -79,8 +79,8 @@ tui_apply_current_layout (bool preserve_cmd_win_size_p)
       tui_win_list[win_type] = nullptr;
 
   /* This should always be made visible by a layout.  */
-  gdb_assert (TUI_CMD_WIN != nullptr);
-  gdb_assert (TUI_CMD_WIN->is_visible ());
+  gdb_assert (tui_cmd_win () != nullptr);
+  gdb_assert (tui_cmd_win ()->is_visible ());
 
   /* Get the new list of currently visible windows.  */
   std::vector<tui_win_info *> new_tui_windows;
@@ -132,7 +132,7 @@ tui_set_layout (tui_layout_split *layout)
 
   std::string new_fingerprint = applied_layout->layout_fingerprint ();
   bool preserve_command_window_size
-    = (TUI_CMD_WIN != nullptr && old_fingerprint == new_fingerprint);
+    = (tui_cmd_win () != nullptr && old_fingerprint == new_fingerprint);
 
   tui_apply_current_layout (preserve_command_window_size);
 }
@@ -233,10 +233,10 @@ void
 tui_regs_layout ()
 {
   /* If there's already a register window, we're done.  */
-  if (TUI_DATA_WIN != nullptr)
+  if (tui_data_win () != nullptr)
     return;
 
-  tui_set_layout (TUI_DISASM_WIN != nullptr
+  tui_set_layout (tui_disasm_win () != nullptr
 		  ? asm_regs_layout
 		  : src_regs_layout);
 }
@@ -261,9 +261,9 @@ tui_remove_some_windows ()
     {
       /* Try leaving the source or disassembly window.  If neither
 	 exists, just do nothing.  */
-      focus = TUI_SRC_WIN;
+      focus = tui_src_win ();
       if (focus == nullptr)
-	focus = TUI_DISASM_WIN;
+	focus = tui_disasm_win ();
       if (focus == nullptr)
 	return;
     }
@@ -817,7 +817,7 @@ tui_layout_split::apply (int x_, int y_, int width_, int height_,
   int prev = -1;
   for (int i = 0; i < m_splits.size (); ++i)
     {
-      bool cmd_win_already_exists = TUI_CMD_WIN != nullptr;
+      bool cmd_win_already_exists = tui_cmd_win () != nullptr;
 
       /* Always call get_sizes, to ensure that the window is
 	 instantiated.  This is a bit gross but less gross than adding
@@ -841,8 +841,8 @@ tui_layout_split::apply (int x_, int y_, int width_, int height_,
 	     that the resizing step, below, does the right thing with
 	     this window.  */
 	  info[i].min_size = (m_vertical
-			      ? TUI_CMD_WIN->height
-			      : TUI_CMD_WIN->width);
+			      ? tui_cmd_win ()->height
+			      : tui_cmd_win ()->width);
 	  info[i].max_size = info[i].min_size;
 	}
 
diff --git a/gdb/tui/tui-layout.h b/gdb/tui/tui-layout.h
index f621f9cea55d..a401efcb8538 100644
--- a/gdb/tui/tui-layout.h
+++ b/gdb/tui/tui-layout.h
@@ -59,8 +59,8 @@ class tui_layout_base
   virtual std::unique_ptr<tui_layout_base> clone () const = 0;
 
   /* Change the size and location of this layout.  When
-     PRESERVE_CMD_WIN_SIZE_P is true the current size of the TUI_CMD_WIN
-     is preserved, otherwise, the TUI_CMD_WIN will resize just like any
+     PRESERVE_CMD_WIN_SIZE_P is true the current size of the command window
+     is preserved, otherwise, the command window will resize just like any
      other window.  */
   virtual void apply (int x, int y, int width, int height,
 		      bool preserve_cmd_win_size_p) = 0;
@@ -350,8 +350,8 @@ extern void tui_regs_layout ();
 extern void tui_remove_some_windows ();
 
 /* Apply the current layout.  When PRESERVE_CMD_WIN_SIZE_P is true the
-   current size of the TUI_CMD_WIN is preserved, otherwise, the TUI_CMD_WIN
-   will resize just like any other window.  */
+   current size of the command window is preserved, otherwise, the command
+   window will resize just like any other window.  */
 extern void tui_apply_current_layout (bool);
 
 /* Adjust the window height of WIN to NEW_HEIGHT.  */
diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index 8db269181237..50708fd383fc 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -515,11 +515,11 @@ tui_reg_command (const char *args, int from_tty)
       /* Make sure the register window is visible.  If not, select an
 	 appropriate layout.  We need to do this before trying to run the
 	 'next' or 'prev' commands.  */
-      if (TUI_DATA_WIN == NULL || !TUI_DATA_WIN->is_visible ())
+      if (tui_data_win () == nullptr || !tui_data_win ()->is_visible ())
 	tui_regs_layout ();
 
       const reggroup *match = nullptr;
-      const reggroup *current_group = TUI_DATA_WIN->get_current_group ();
+      const reggroup *current_group = tui_data_win ()->get_current_group ();
       if (strncmp (args, "next", len) == 0)
 	match = tui_reg_next (current_group, gdbarch);
       else if (strncmp (args, "prev", len) == 0)
@@ -543,7 +543,7 @@ tui_reg_command (const char *args, int from_tty)
       if (match == NULL)
 	error (_("unknown register group '%s'"), args);
 
-      TUI_DATA_WIN->set_register_group (match);
+      tui_data_win ()->set_register_group (match);
     }
   else
     {
diff --git a/gdb/tui/tui-regs.h b/gdb/tui/tui-regs.h
index 07b951316b6b..4a799e6b2f68 100644
--- a/gdb/tui/tui-regs.h
+++ b/gdb/tui/tui-regs.h
@@ -145,4 +145,12 @@ struct tui_data_window : public tui_win_info
   gdbarch *m_gdbarch = nullptr;
 };
 
+/* Return the instance of the registers window.  */
+
+inline tui_data_window *
+tui_data_win ()
+{
+  return gdb::checked_static_cast<tui_data_window *> (tui_win_list[DATA_WIN]);
+}
+
 #endif /* TUI_TUI_REGS_H */
diff --git a/gdb/tui/tui-source.h b/gdb/tui/tui-source.h
index 103b11e72737..9a72ed90b476 100644
--- a/gdb/tui/tui-source.h
+++ b/gdb/tui/tui-source.h
@@ -82,4 +82,12 @@ struct tui_source_window : public tui_source_window_base
   gdb::unique_xmalloc_ptr<char> m_fullname;
 };
 
+/* Return the instance of the source window.  */
+
+inline tui_source_window *
+tui_src_win ()
+{
+  return gdb::checked_static_cast<tui_source_window *> (tui_win_list[SRC_WIN]);
+}
+
 #endif /* TUI_TUI_SOURCE_H */
diff --git a/gdb/tui/tui-status.c b/gdb/tui/tui-status.c
index c7750af6ea0b..de754cbe6a6c 100644
--- a/gdb/tui/tui-status.c
+++ b/gdb/tui/tui-status.c
@@ -315,7 +315,7 @@ void
 tui_show_status_content ()
 {
   if (tui_is_window_visible (STATUS_WIN))
-    TUI_STATUS_WIN->rerender ();
+    tui_status_win ()->rerender ();
 }
 
 /* Command to update the display with the current execution point.  */
diff --git a/gdb/tui/tui-status.h b/gdb/tui/tui-status.h
index 0af446641042..f7cd2d4191c2 100644
--- a/gdb/tui/tui-status.h
+++ b/gdb/tui/tui-status.h
@@ -50,6 +50,14 @@ struct tui_status_window
   std::string make_status_line () const;
 };
 
+/* Return the instance of the status window.  */
+
+inline tui_status_window *
+tui_status_win ()
+{
+  return dynamic_cast<tui_status_window *> (tui_win_list[STATUS_WIN]);
+}
+
 extern void tui_show_status_content (void);
 extern void tui_show_frame_info (const frame_info_ptr &);
 
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 85b62321ca9f..e79e7b67c6de 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -202,10 +202,10 @@ static void
 set_style_tui_current_position (const char *ignore, int from_tty,
 				cmd_list_element *c)
 {
-  if (TUI_SRC_WIN != nullptr)
-    TUI_SRC_WIN->refill ();
-  if (TUI_DISASM_WIN != nullptr)
-    TUI_DISASM_WIN->refill ();
+  if (tui_src_win () != nullptr)
+    tui_src_win ()->refill ();
+  if (tui_disasm_win () != nullptr)
+    tui_disasm_win ()->refill ();
 }
 
 /* Tui internal configuration variables.  These variables are updated
@@ -423,8 +423,8 @@ tui_update_gdb_sizes (void)
 
   if (tui_active)
     {
-      width = TUI_CMD_WIN->width;
-      height = TUI_CMD_WIN->height;
+      width = tui_cmd_win ()->width;
+      height = tui_cmd_win ()->height;
     }
   else
     {
@@ -512,7 +512,7 @@ tui_resize_all (void)
       resize_term (screenheight, screenwidth);
 #endif      
       /* Turn keypad off while we resize.  */
-      keypad (TUI_CMD_WIN->handle.get (), FALSE);
+      keypad (tui_cmd_win ()->handle.get (), FALSE);
       tui_update_gdb_sizes ();
       tui_set_term_height_to (screenheight);
       tui_set_term_width_to (screenwidth);
@@ -525,7 +525,7 @@ tui_resize_all (void)
 	 window to resize proportionately with containing terminal, rather
 	 than maintaining a fixed size.  */
       tui_apply_current_layout (false); /* Turn keypad back on.  */
-      keypad (TUI_CMD_WIN->handle.get (), TRUE);
+      keypad (tui_cmd_win ()->handle.get (), TRUE);
     }
 }
 
@@ -861,8 +861,8 @@ static void
 tui_set_compact_source (const char *ignore, int from_tty,
 			struct cmd_list_element *c)
 {
-  if (TUI_SRC_WIN != nullptr)
-    TUI_SRC_WIN->refill ();
+  if (tui_src_win () != nullptr)
+    tui_src_win ()->refill ();
 }
 
 /* Callback for "show tui compact-source".  */
@@ -1080,7 +1080,7 @@ parse_scrolling_args (const char *arg,
 		error (_("Unrecognized window `%s'"), wname);
 	      if (!(*win_to_scroll)->is_visible ())
 		error (_("Window is not visible"));
-	      else if (*win_to_scroll == TUI_CMD_WIN)
+	      else if (*win_to_scroll == tui_cmd_win ())
 		*win_to_scroll = *(tui_source_windows ().begin ());
 	    }
 	}
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index 61c8e00589db..e0f856cdbfc9 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -465,7 +465,7 @@ tui_source_window_base::rerender ()
       struct gdbarch *gdbarch = get_frame_arch (frame);
 
       struct symtab *s = find_pc_line_symtab (get_frame_pc (frame));
-      if (this != TUI_SRC_WIN)
+      if (this != tui_src_win ())
 	find_line_pc (s, cursal.line, &cursal.pc);
 
       /* This centering code is copied from tui_source_window::maybe_update.
@@ -498,7 +498,7 @@ tui_source_window_base::refill ()
 {
   symtab_and_line sal {};
 
-  if (this == TUI_SRC_WIN)
+  if (this == tui_src_win ())
     {
       sal = get_current_source_symtab_and_line ();
       if (sal.symtab == NULL)
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index cb21d68e9b62..008041de0b77 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -469,9 +469,9 @@ tui_enable (void)
 
       tui_show_frame_info (deprecated_safe_get_selected_frame ());
       tui_set_initial_layout ();
-      tui_set_win_focus_to (TUI_SRC_WIN);
-      keypad (TUI_CMD_WIN->handle.get (), TRUE);
-      wrefresh (TUI_CMD_WIN->handle.get ());
+      tui_set_win_focus_to (tui_src_win ());
+      keypad (tui_cmd_win ()->handle.get (), TRUE);
+      wrefresh (tui_cmd_win ()->handle.get ());
       tui_finish_init = false;
     }
   else
@@ -594,11 +594,11 @@ bool
 tui_get_command_dimension (unsigned int *width, 
 			   unsigned int *height)
 {
-  if (!tui_active || (TUI_CMD_WIN == NULL))
+  if (!tui_active || (tui_cmd_win () == NULL))
     return false;
   
-  *width = TUI_CMD_WIN->width;
-  *height = TUI_CMD_WIN->height;
+  *width = tui_cmd_win ()->width;
+  *height = tui_cmd_win ()->height;
   return true;
 }
 

base-commit: 6f69575237fd63559fc284bb2b9ce4bf5d12bf16
-- 
2.45.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/3] gdb: add IWYU export pragams to gdb_curses.h
  2024-05-31  2:54 [PATCH 1/3] gdb/tui: change some macros to functions Simon Marchi
@ 2024-05-31  2:54 ` Simon Marchi
  2024-06-03 14:56   ` Tom Tromey
  2024-05-31  2:54 ` [PATCH 3/3] gdb/tui: cleanup includes Simon Marchi
  2024-06-03 14:55 ` [PATCH 1/3] gdb/tui: change some macros to functions Tom Tromey
  2 siblings, 1 reply; 12+ messages in thread
From: Simon Marchi @ 2024-05-31  2:54 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

From: Simon Marchi <simon.marchi@efficios.com>

It seems like gdb_curses.h is included whenever we want to access
ncurses functionality, instead of including directly ncurses.h.  As a
result, clangd often erroneously shows that gdb_curses.h inclusions are
unused.

By adding those pragmas, clangd (and the include-what-you-use tool)
understands that gdb_curses.h is a valid provider for whatever these
ncurses.h files provide.

Change-Id: Ia8acd761dae1577f7151d5fb558f35514b4e4ea2
---
 gdb/gdb_curses.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/gdb/gdb_curses.h b/gdb/gdb_curses.h
index c7ee862cd672..8eb2e8c750ab 100644
--- a/gdb/gdb_curses.h
+++ b/gdb/gdb_curses.h
@@ -40,21 +40,21 @@
 #define NCURSES_NOMACROS
 
 #if defined (HAVE_NCURSESW_NCURSES_H)
-#include <ncursesw/ncurses.h>
+#include <ncursesw/ncurses.h> /* IWYU pragma: export */
 #elif defined (HAVE_NCURSES_NCURSES_H)
-#include <ncurses/ncurses.h>
+#include <ncurses/ncurses.h> /* IWYU pragma: export */
 #elif defined (HAVE_NCURSES_H)
-#include <ncurses.h>
+#include <ncurses.h> /* IWYU pragma: export */
 #elif defined (HAVE_CURSESX_H)
-#include <cursesX.h>
+#include <cursesX.h> /* IWYU pragma: export */
 #elif defined (HAVE_CURSES_H)
-#include <curses.h>
+#include <curses.h> /* IWYU pragma: export */
 #endif
 
 #if defined (HAVE_NCURSES_TERM_H)
-#include <ncurses/term.h>
+#include <ncurses/term.h> /* IWYU pragma: export */
 #elif defined (HAVE_TERM_H)
-#include <term.h>
+#include <term.h> /* IWYU pragma: export */
 #else
 /* On MinGW, a real termcap library is usually not present.  Stub versions
    of the termcap functions will be built from stub-termcap.c.  Readline
-- 
2.45.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 3/3] gdb/tui: cleanup includes
  2024-05-31  2:54 [PATCH 1/3] gdb/tui: change some macros to functions Simon Marchi
  2024-05-31  2:54 ` [PATCH 2/3] gdb: add IWYU export pragams to gdb_curses.h Simon Marchi
@ 2024-05-31  2:54 ` Simon Marchi
  2024-06-03 14:57   ` Tom Tromey
  2024-06-03 14:55 ` [PATCH 1/3] gdb/tui: change some macros to functions Tom Tromey
  2 siblings, 1 reply; 12+ messages in thread
From: Simon Marchi @ 2024-05-31  2:54 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

Remove includes reported as unused by clangd.  Then, add any includes
necessary to get rid of errors (includes possibly relying on previous
includes)..

I didn't remove the includes of gdb-safe-ctypes.h, because it appears to
do some some preprocessor magic, redefining standard macros.  I'm afraid
that removing these includes could change the behavior unintentionally.

Change-Id: I4c5b652355c3bbce022fe0d447a72dc4e1d17d34
---
 gdb/tui/tui-command.c    |  4 ----
 gdb/tui/tui-data.c       |  2 --
 gdb/tui/tui-data.h       |  2 --
 gdb/tui/tui-disasm.c     |  6 ------
 gdb/tui/tui-disasm.h     |  1 +
 gdb/tui/tui-file.c       |  1 -
 gdb/tui/tui-hooks.c      | 12 ------------
 gdb/tui/tui-interp.c     |  5 -----
 gdb/tui/tui-io.c         |  1 -
 gdb/tui/tui-layout.c     |  5 -----
 gdb/tui/tui-location.c   |  2 --
 gdb/tui/tui-location.h   |  5 -----
 gdb/tui/tui-regs.c       |  6 ------
 gdb/tui/tui-regs.h       |  1 +
 gdb/tui/tui-source.c     |  6 ------
 gdb/tui/tui-source.h     |  1 +
 gdb/tui/tui-status.c     |  4 ----
 gdb/tui/tui-win.c        | 10 +---------
 gdb/tui/tui-wingeneral.c |  2 --
 gdb/tui/tui-winsource.c  |  4 +---
 gdb/tui/tui-winsource.h  |  1 +
 gdb/tui/tui.c            |  6 ------
 22 files changed, 6 insertions(+), 81 deletions(-)

diff --git a/gdb/tui/tui-command.c b/gdb/tui/tui-command.c
index d36c1372fb88..b9bc19ee9eeb 100644
--- a/gdb/tui/tui-command.c
+++ b/gdb/tui/tui-command.c
@@ -19,10 +19,6 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#include "tui/tui.h"
-#include "tui/tui-data.h"
-#include "tui/tui-win.h"
-#include "tui/tui-io.h"
 #include "tui/tui-command.h"
 #include "tui/tui-wingeneral.h"
 
diff --git a/gdb/tui/tui-data.c b/gdb/tui/tui-data.c
index 462771ca1637..962dbc511607 100644
--- a/gdb/tui/tui-data.c
+++ b/gdb/tui/tui-data.c
@@ -19,12 +19,10 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#include "symtab.h"
 #include "tui/tui.h"
 #include "tui/tui-data.h"
 #include "tui/tui-win.h"
 #include "tui/tui-wingeneral.h"
-#include "tui/tui-winsource.h"
 #include "tui/tui-status.h"
 #include "gdb_curses.h"
 #include <algorithm>
diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h
index d1c855434671..0b55f1fb951b 100644
--- a/gdb/tui/tui-data.h
+++ b/gdb/tui/tui-data.h
@@ -24,8 +24,6 @@
 
 #include "tui/tui.h"
 #include "gdb_curses.h"
-#include "observable.h"
-#include "gdbsupport/gdb-checked-static-cast.h"
 
 /* A deleter that calls delwin.  */
 struct curses_deleter
diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c
index 24a5044ed379..0727d3a90a82 100644
--- a/gdb/tui/tui-disasm.c
+++ b/gdb/tui/tui-disasm.c
@@ -29,13 +29,9 @@
 #include "tui/tui.h"
 #include "tui/tui-command.h"
 #include "tui/tui-data.h"
-#include "tui/tui-win.h"
-#include "tui/tui-layout.h"
 #include "tui/tui-winsource.h"
 #include "tui/tui-status.h"
-#include "tui/tui-file.h"
 #include "tui/tui-disasm.h"
-#include "tui/tui-source.h"
 #include "progspace.h"
 #include "objfiles.h"
 #include "cli/cli-style.h"
@@ -43,8 +39,6 @@
 #include "gdbsupport/selftest.h"
 #include "inferior.h"
 
-#include "gdb_curses.h"
-
 struct tui_asm_line
 {
   CORE_ADDR addr;
diff --git a/gdb/tui/tui-disasm.h b/gdb/tui/tui-disasm.h
index 4a7735450ce4..c5e8eb483d19 100644
--- a/gdb/tui/tui-disasm.h
+++ b/gdb/tui/tui-disasm.h
@@ -22,6 +22,7 @@
 #ifndef TUI_TUI_DISASM_H
 #define TUI_TUI_DISASM_H
 
+#include "gdbsupport/gdb-checked-static-cast.h"
 #include "tui/tui.h"
 #include "tui/tui-data.h"
 #include "tui-winsource.h"
diff --git a/gdb/tui/tui-file.c b/gdb/tui/tui-file.c
index a0d72a0fbd2e..78781909b1c3 100644
--- a/gdb/tui/tui-file.c
+++ b/gdb/tui/tui-file.c
@@ -19,7 +19,6 @@
 #include "tui/tui-file.h"
 #include "tui/tui-io.h"
 #include "tui/tui-command.h"
-#include "tui.h"
 
 void
 tui_file::puts (const char *linebuffer)
diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
index 9dcf789ccae9..e6673ab650d8 100644
--- a/gdb/tui/tui-hooks.c
+++ b/gdb/tui/tui-hooks.c
@@ -19,18 +19,11 @@
 
 #include "symtab.h"
 #include "inferior.h"
-#include "command.h"
-#include "bfd.h"
 #include "symfile.h"
 #include "objfiles.h"
 #include "target.h"
-#include "gdbcore.h"
-#include "gdbsupport/event-loop.h"
-#include "event-top.h"
 #include "frame.h"
 #include "breakpoint.h"
-#include "ui-out.h"
-#include "top.h"
 #include "observable.h"
 #include "source.h"
 #include <unistd.h>
@@ -38,16 +31,11 @@
 
 #include "tui/tui.h"
 #include "tui/tui-hooks.h"
-#include "tui/tui-data.h"
 #include "tui/tui-layout.h"
-#include "tui/tui-io.h"
 #include "tui/tui-regs.h"
-#include "tui/tui-win.h"
 #include "tui/tui-status.h"
 #include "tui/tui-winsource.h"
 
-#include "gdb_curses.h"
-
 static void
 tui_new_objfile_hook (struct objfile* objfile)
 {
diff --git a/gdb/tui/tui-interp.c b/gdb/tui/tui-interp.c
index 7ebaf8f83f74..25761befc651 100644
--- a/gdb/tui/tui-interp.c
+++ b/gdb/tui/tui-interp.c
@@ -21,16 +21,11 @@
 #include "interps.h"
 #include "ui.h"
 #include "event-top.h"
-#include "gdbsupport/event-loop.h"
 #include "ui-out.h"
 #include "cli-out.h"
-#include "tui/tui-data.h"
 #include "tui/tui-win.h"
 #include "tui/tui.h"
 #include "tui/tui-io.h"
-#include "infrun.h"
-#include "observable.h"
-#include "gdbthread.h"
 #include "inferior.h"
 #include "main.h"
 
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 461ac663279b..c715b36d847e 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -22,7 +22,6 @@
 #include "target.h"
 #include "gdbsupport/event-loop.h"
 #include "event-top.h"
-#include "command.h"
 #include "top.h"
 #include "ui.h"
 #include "tui/tui.h"
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 3cb5a05b626a..665b52115598 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -19,15 +19,11 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#include "arch-utils.h"
 #include "command.h"
 #include "symtab.h"
 #include "frame.h"
-#include "source.h"
-#include "cli/cli-cmds.h"
 #include "cli/cli-decode.h"
 #include "cli/cli-utils.h"
-#include <ctype.h>
 #include <unordered_set>
 
 #include "tui/tui.h"
@@ -37,7 +33,6 @@
 #include "tui/tui-status.h"
 #include "tui/tui-regs.h"
 #include "tui/tui-win.h"
-#include "tui/tui-winsource.h"
 #include "tui/tui-disasm.h"
 #include "tui/tui-layout.h"
 #include "tui/tui-source.h"
diff --git a/gdb/tui/tui-location.c b/gdb/tui/tui-location.c
index 7e8f769d8047..8e63d4f2c410 100644
--- a/gdb/tui/tui-location.c
+++ b/gdb/tui/tui-location.c
@@ -15,9 +15,7 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#include "tui/tui.h"
 #include "tui/tui-status.h"
-#include "tui/tui-data.h"
 #include "tui/tui-location.h"
 #include "symtab.h"
 #include "source.h"
diff --git a/gdb/tui/tui-location.h b/gdb/tui/tui-location.h
index db558a4e1473..ec05f4570692 100644
--- a/gdb/tui/tui-location.h
+++ b/gdb/tui/tui-location.h
@@ -18,11 +18,6 @@
 #ifndef TUI_TUI_LOCATION_H
 #define TUI_TUI_LOCATION_H
 
-#include "tui/tui.h"
-#include "tui/tui.h"
-#include "gdb_curses.h"
-#include "observable.h"
-
 /* Class used to track the current location that the TUI is displaying.  An
    instance of this class will be created; as events occur within GDB the
    location information within this instance will be updated.  As windows
diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index 50708fd383fc..43b519ac127c 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -21,10 +21,7 @@
 
 #include "arch-utils.h"
 #include "tui/tui.h"
-#include "tui/tui-data.h"
 #include "symtab.h"
-#include "gdbtypes.h"
-#include "cli/cli-cmds.h"
 #include "frame.h"
 #include "regcache.h"
 #include "inferior.h"
@@ -32,11 +29,8 @@
 #include "tui/tui-layout.h"
 #include "tui/tui-win.h"
 #include "tui/tui-wingeneral.h"
-#include "tui/tui-file.h"
 #include "tui/tui-regs.h"
-#include "tui/tui-io.h"
 #include "reggroups.h"
-#include "valprint.h"
 #include "completer.h"
 
 #include "gdb_curses.h"
diff --git a/gdb/tui/tui-regs.h b/gdb/tui/tui-regs.h
index 4a799e6b2f68..69ba10724e7c 100644
--- a/gdb/tui/tui-regs.h
+++ b/gdb/tui/tui-regs.h
@@ -22,6 +22,7 @@
 #ifndef TUI_TUI_REGS_H
 #define TUI_TUI_REGS_H
 
+#include "gdbsupport/gdb-checked-static-cast.h"
 #include "tui/tui-data.h"
 #include "reggroups.h"
 
diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c
index 5c17aba3a1a2..444c0f7a90e6 100644
--- a/gdb/tui/tui-source.c
+++ b/gdb/tui/tui-source.c
@@ -20,7 +20,6 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include <math.h>
-#include <ctype.h>
 #include "symtab.h"
 #include "frame.h"
 #include "breakpoint.h"
@@ -28,16 +27,11 @@
 #include "objfiles.h"
 #include "filenames.h"
 #include "source-cache.h"
-
-#include "tui/tui.h"
-#include "tui/tui-data.h"
-#include "tui/tui-io.h"
 #include "tui/tui-status.h"
 #include "tui/tui-win.h"
 #include "tui/tui-winsource.h"
 #include "tui/tui-source.h"
 #include "tui/tui-location.h"
-#include "gdb_curses.h"
 
 /* Function to display source in the source window.  */
 bool
diff --git a/gdb/tui/tui-source.h b/gdb/tui/tui-source.h
index 9a72ed90b476..f32167f42246 100644
--- a/gdb/tui/tui-source.h
+++ b/gdb/tui/tui-source.h
@@ -22,6 +22,7 @@
 #ifndef TUI_TUI_SOURCE_H
 #define TUI_TUI_SOURCE_H
 
+#include "gdbsupport/gdb-checked-static-cast.h"
 #include "tui/tui-data.h"
 #include "tui-winsource.h"
 
diff --git a/gdb/tui/tui-status.c b/gdb/tui/tui-status.c
index de754cbe6a6c..523941576118 100644
--- a/gdb/tui/tui-status.c
+++ b/gdb/tui/tui-status.c
@@ -20,21 +20,17 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "symtab.h"
-#include "breakpoint.h"
 #include "frame.h"
 #include "command.h"
 #include "inferior.h"
 #include "target.h"
 #include "top.h"
 #include "gdb-demangle.h"
-#include "source.h"
 #include "tui/tui.h"
 #include "tui/tui-data.h"
 #include "tui/tui-status.h"
 #include "tui/tui-wingeneral.h"
-#include "tui/tui-source.h"
 #include "tui/tui-winsource.h"
-#include "tui/tui-file.h"
 #include "tui/tui-location.h"
 
 #include "gdb_curses.h"
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index e79e7b67c6de..2f793e264e1c 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -26,14 +26,10 @@
 
 #include "command.h"
 #include "symtab.h"
-#include "breakpoint.h"
 #include "frame.h"
 #include "cli/cli-cmds.h"
 #include "cli/cli-style.h"
-#include "top.h"
-#include "source.h"
-#include "gdbsupport/event-loop.h"
-#include "async-event.h"
+#include "ui-out.h"
 #include "utils.h"
 
 #include "tui/tui.h"
@@ -42,8 +38,6 @@
 #include "tui/tui-data.h"
 #include "tui/tui-layout.h"
 #include "tui/tui-wingeneral.h"
-#include "tui/tui-status.h"
-#include "tui/tui-regs.h"
 #include "tui/tui-disasm.h"
 #include "tui/tui-source.h"
 #include "tui/tui-winsource.h"
@@ -54,8 +48,6 @@
 #include "readline/readline.h"
 #include <string_view>
 
-#include <signal.h>
-
 static void tui_set_tab_width_command (const char *, int);
 static void tui_refresh_all_command (const char *, int);
 static void tui_all_windows_info (const char *, int);
diff --git a/gdb/tui/tui-wingeneral.c b/gdb/tui/tui-wingeneral.c
index 6260d08cd3d7..11530472687e 100644
--- a/gdb/tui/tui-wingeneral.c
+++ b/gdb/tui/tui-wingeneral.c
@@ -19,12 +19,10 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#include "tui/tui.h"
 #include "tui/tui-data.h"
 #include "tui/tui-io.h"
 #include "tui/tui-wingeneral.h"
 #include "tui/tui-win.h"
-#include "tui/tui-status.h"
 #include "cli/cli-style.h"
 
 #include "gdb_curses.h"
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index e0f856cdbfc9..074069e34ea8 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -19,14 +19,13 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#include <ctype.h>
+#include "observable.h"
 #include "symtab.h"
 #include "frame.h"
 #include "breakpoint.h"
 #include "value.h"
 #include "source.h"
 #include "objfiles.h"
-#include "filenames.h"
 #include "gdbsupport/gdb-safe-ctype.h"
 
 #include "tui/tui.h"
@@ -34,7 +33,6 @@
 #include "tui/tui-io.h"
 #include "tui/tui-status.h"
 #include "tui/tui-win.h"
-#include "tui/tui-wingeneral.h"
 #include "tui/tui-winsource.h"
 #include "tui/tui-source.h"
 #include "tui/tui-disasm.h"
diff --git a/gdb/tui/tui-winsource.h b/gdb/tui/tui-winsource.h
index 646597306f26..29828c121bd6 100644
--- a/gdb/tui/tui-winsource.h
+++ b/gdb/tui/tui-winsource.h
@@ -22,6 +22,7 @@
 #ifndef TUI_TUI_WINSOURCE_H
 #define TUI_TUI_WINSOURCE_H
 
+#include "gdbsupport/observable.h"
 #include "tui/tui-data.h"
 #include "symtab.h"
 
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index 008041de0b77..85a35db7cca3 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -27,7 +27,6 @@
 #include "tui/tui-data.h"
 #include "tui/tui-layout.h"
 #include "tui/tui-io.h"
-#include "tui/tui-regs.h"
 #include "tui/tui-status.h"
 #include "tui/tui-win.h"
 #include "tui/tui-wingeneral.h"
@@ -35,18 +34,13 @@
 #include "tui/tui-source.h"
 #include "target.h"
 #include "frame.h"
-#include "breakpoint.h"
 #include "inferior.h"
 #include "symtab.h"
-#include "source.h"
 #include "terminal.h"
 #include "top.h"
 #include "ui.h"
 
-#include <ctype.h>
-#include <signal.h>
 #include <fcntl.h>
-#include <setjmp.h>
 
 #include "gdb_curses.h"
 #include "interps.h"
-- 
2.45.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] gdb/tui: change some macros to functions
  2024-05-31  2:54 [PATCH 1/3] gdb/tui: change some macros to functions Simon Marchi
  2024-05-31  2:54 ` [PATCH 2/3] gdb: add IWYU export pragams to gdb_curses.h Simon Marchi
  2024-05-31  2:54 ` [PATCH 3/3] gdb/tui: cleanup includes Simon Marchi
@ 2024-06-03 14:55 ` Tom Tromey
  2 siblings, 0 replies; 12+ messages in thread
From: Tom Tromey @ 2024-06-03 14:55 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches, Simon Marchi

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> From: Simon Marchi <simon.marchi@efficios.com>
Simon> Change the `TUI_*` macros to access known windows to functions.  Define
Simon> them in their respective files, because trying to define them in
Simon> tui-data.h would end up causing include cycles.

Simon> This makes static analysis (detection of unused include files in this
Simon> case) more accurate, and I think in general we should avoid hiding
Simon> code behind macros if not necessary.

Ok.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] gdb: add IWYU export pragams to gdb_curses.h
  2024-05-31  2:54 ` [PATCH 2/3] gdb: add IWYU export pragams to gdb_curses.h Simon Marchi
@ 2024-06-03 14:56   ` Tom Tromey
  2024-06-08  3:07     ` Simon Marchi
  0 siblings, 1 reply; 12+ messages in thread
From: Tom Tromey @ 2024-06-03 14:56 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches, Simon Marchi

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> From: Simon Marchi <simon.marchi@efficios.com>
Simon> It seems like gdb_curses.h is included whenever we want to access
Simon> ncurses functionality, instead of including directly ncurses.h.  As a
Simon> result, clangd often erroneously shows that gdb_curses.h inclusions are
Simon> unused.

Simon> By adding those pragmas, clangd (and the include-what-you-use tool)
Simon> understands that gdb_curses.h is a valid provider for whatever these
Simon> ncurses.h files provide.

I wish this were less ugly, but anyway it's ok.
Approved-By: Tom Tromey <tom@tromey.com>

This isn't the only place this idiom is used in gdb, not to mention
gnulib, which basically only exists to do this.

Tom

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] gdb/tui: cleanup includes
  2024-05-31  2:54 ` [PATCH 3/3] gdb/tui: cleanup includes Simon Marchi
@ 2024-06-03 14:57   ` Tom Tromey
  2024-06-08 10:14     ` Tom de Vries
  2024-06-09  5:51     ` Aditya Kamath1
  0 siblings, 2 replies; 12+ messages in thread
From: Tom Tromey @ 2024-06-03 14:57 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> Remove includes reported as unused by clangd.  Then, add any includes
Simon> necessary to get rid of errors (includes possibly relying on previous
Simon> includes)..

Simon> I didn't remove the includes of gdb-safe-ctypes.h, because it appears to
Simon> do some some preprocessor magic, redefining standard macros.  I'm afraid
Simon> that removing these includes could change the behavior unintentionally.

It's never been super clear when gdb should use the libiberty stuff and
when it should not.

Approved-By: Tom Tromey <tom@tromey.com>

Tom

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] gdb: add IWYU export pragams to gdb_curses.h
  2024-06-03 14:56   ` Tom Tromey
@ 2024-06-08  3:07     ` Simon Marchi
  0 siblings, 0 replies; 12+ messages in thread
From: Simon Marchi @ 2024-06-08  3:07 UTC (permalink / raw)
  To: Tom Tromey, Simon Marchi; +Cc: gdb-patches

On 2024-06-03 10:56, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
> Simon> From: Simon Marchi <simon.marchi@efficios.com>
> Simon> It seems like gdb_curses.h is included whenever we want to access
> Simon> ncurses functionality, instead of including directly ncurses.h.  As a
> Simon> result, clangd often erroneously shows that gdb_curses.h inclusions are
> Simon> unused.
> 
> Simon> By adding those pragmas, clangd (and the include-what-you-use tool)
> Simon> understands that gdb_curses.h is a valid provider for whatever these
> Simon> ncurses.h files provide.
> 
> I wish this were less ugly, but anyway it's ok.
> Approved-By: Tom Tromey <tom@tromey.com>

I don't really know how they could be made less ugly... but at least
they seem to work fine.

> This isn't the only place this idiom is used in gdb, not to mention
> gnulib, which basically only exists to do this.

So far I didn't hit any problem like this with gnulib, hopefully it
stays that way.

Simon

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] gdb/tui: cleanup includes
  2024-06-03 14:57   ` Tom Tromey
@ 2024-06-08 10:14     ` Tom de Vries
  2024-06-09 14:14       ` Simon Marchi
  2024-06-09  5:51     ` Aditya Kamath1
  1 sibling, 1 reply; 12+ messages in thread
From: Tom de Vries @ 2024-06-08 10:14 UTC (permalink / raw)
  To: Tom Tromey, Simon Marchi; +Cc: gdb-patches

On 6/3/24 16:57, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
> Simon> Remove includes reported as unused by clangd.  Then, add any includes
> Simon> necessary to get rid of errors (includes possibly relying on previous
> Simon> includes)..
> 
> Simon> I didn't remove the includes of gdb-safe-ctypes.h, because it appears to
> Simon> do some some preprocessor magic, redefining standard macros.  I'm afraid
> Simon> that removing these includes could change the behavior unintentionally.
> 
> It's never been super clear when gdb should use the libiberty stuff and
> when it should not.
> 
> Approved-By: Tom Tromey <tom@tromey.com>

This commit introduced a regression for me:
...
Running /data/vries/gdb/src/gdb/testsuite/gdb.python/tui-window.exp ...
WARNING: timeout in accept_gdb_output
WARNING: timeout in accept_gdb_output
FAIL: gdb.python/tui-window.exp: Window was updated
...

Filed here (  https://sourceware.org/bugzilla/show_bug.cgi?id=31865 ) 
with gdb.log attached.

Thanks,
- Tom


^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH 3/3] gdb/tui: cleanup includes
  2024-06-03 14:57   ` Tom Tromey
  2024-06-08 10:14     ` Tom de Vries
@ 2024-06-09  5:51     ` Aditya Kamath1
  1 sibling, 0 replies; 12+ messages in thread
From: Aditya Kamath1 @ 2024-06-09  5:51 UTC (permalink / raw)
  To: Tom Tromey, Simon Marchi; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1280 bytes --]

Respected community members,

Hi,

Is there any work still going on here or anything pending from this patch?

It caused a build break in AIX.

I have sent a patch to fix the same. One of the includes is still required in AIX. (Did not test in other targets).

Kindly let me know if any information is needed so that we can fix this.

Have a nice day ahead.

Thanks and regards,
Aditya.
From: Tom Tromey <tom@tromey.com>
Date: Monday, 3 June 2024 at 8:27 PM
To: Simon Marchi <simon.marchi@polymtl.ca>
Cc: gdb-patches@sourceware.org <gdb-patches@sourceware.org>
Subject: [EXTERNAL] Re: [PATCH 3/3] gdb/tui: cleanup includes
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> Remove includes reported as unused by clangd.  Then, add any includes
Simon> necessary to get rid of errors (includes possibly relying on previous
Simon> includes)..

Simon> I didn't remove the includes of gdb-safe-ctypes.h, because it appears to
Simon> do some some preprocessor magic, redefining standard macros.  I'm afraid
Simon> that removing these includes could change the behavior unintentionally.

It's never been super clear when gdb should use the libiberty stuff and
when it should not.

Approved-By: Tom Tromey <tom@tromey.com>

Tom

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] gdb/tui: cleanup includes
  2024-06-08 10:14     ` Tom de Vries
@ 2024-06-09 14:14       ` Simon Marchi
  2024-06-10  8:44         ` Tom de Vries
  0 siblings, 1 reply; 12+ messages in thread
From: Simon Marchi @ 2024-06-09 14:14 UTC (permalink / raw)
  To: Tom de Vries, Tom Tromey; +Cc: gdb-patches



On 2024-06-08 06:14, Tom de Vries wrote:
> On 6/3/24 16:57, Tom Tromey wrote:
>>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
>>
>> Simon> Remove includes reported as unused by clangd.  Then, add any includes
>> Simon> necessary to get rid of errors (includes possibly relying on previous
>> Simon> includes)..
>>
>> Simon> I didn't remove the includes of gdb-safe-ctypes.h, because it appears to
>> Simon> do some some preprocessor magic, redefining standard macros.  I'm afraid
>> Simon> that removing these includes could change the behavior unintentionally.
>>
>> It's never been super clear when gdb should use the libiberty stuff and
>> when it should not.
>>
>> Approved-By: Tom Tromey <tom@tromey.com>
> 
> This commit introduced a regression for me:
> ...
> Running /data/vries/gdb/src/gdb/testsuite/gdb.python/tui-window.exp ...
> WARNING: timeout in accept_gdb_output
> WARNING: timeout in accept_gdb_output
> FAIL: gdb.python/tui-window.exp: Window was updated
> ...
> 
> Filed here (  https://sourceware.org/bugzilla/show_bug.cgi?id=31865 ) with gdb.log attached.
> 
> Thanks,
> - Tom
> 

Sorry, I messed up. See the patch here:

https://inbox.sourceware.org/gdb-patches/20240609141034.238889-1-simon.marchi@polymtl.ca/T/#u

I'm currently on leave and won't necessarily be near a computer all day, so if you think this
patch is ok, feel free to push it on my behalf in order to fix the build as quickly as possible.

Thanks,

Simon

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] gdb/tui: cleanup includes
  2024-06-09 14:14       ` Simon Marchi
@ 2024-06-10  8:44         ` Tom de Vries
  2024-06-12  4:29           ` Aditya Kamath1
  0 siblings, 1 reply; 12+ messages in thread
From: Tom de Vries @ 2024-06-10  8:44 UTC (permalink / raw)
  To: Simon Marchi, Tom Tromey; +Cc: gdb-patches

On 6/9/24 16:14, Simon Marchi wrote:
> 
> 
> On 2024-06-08 06:14, Tom de Vries wrote:
>> On 6/3/24 16:57, Tom Tromey wrote:
>>>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
>>>
>>> Simon> Remove includes reported as unused by clangd.  Then, add any includes
>>> Simon> necessary to get rid of errors (includes possibly relying on previous
>>> Simon> includes)..
>>>
>>> Simon> I didn't remove the includes of gdb-safe-ctypes.h, because it appears to
>>> Simon> do some some preprocessor magic, redefining standard macros.  I'm afraid
>>> Simon> that removing these includes could change the behavior unintentionally.
>>>
>>> It's never been super clear when gdb should use the libiberty stuff and
>>> when it should not.
>>>
>>> Approved-By: Tom Tromey <tom@tromey.com>
>>
>> This commit introduced a regression for me:
>> ...
>> Running /data/vries/gdb/src/gdb/testsuite/gdb.python/tui-window.exp ...
>> WARNING: timeout in accept_gdb_output
>> WARNING: timeout in accept_gdb_output
>> FAIL: gdb.python/tui-window.exp: Window was updated
>> ...
>>
>> Filed here (  https://sourceware.org/bugzilla/show_bug.cgi?id=31865 ) with gdb.log attached.
>>
>> Thanks,
>> - Tom
>>
> 
> Sorry, I messed up. See the patch here:
> 
> https://inbox.sourceware.org/gdb-patches/20240609141034.238889-1-simon.marchi@polymtl.ca/T/#u
> 
> I'm currently on leave and won't necessarily be near a computer all day, so if you think this
> patch is ok, feel free to push it on my behalf in order to fix the build as quickly as possible.
> 

Thanks for the quick fix, pushed.

- Tom


^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH 3/3] gdb/tui: cleanup includes
  2024-06-10  8:44         ` Tom de Vries
@ 2024-06-12  4:29           ` Aditya Kamath1
  0 siblings, 0 replies; 12+ messages in thread
From: Aditya Kamath1 @ 2024-06-12  4:29 UTC (permalink / raw)
  To: Tom de Vries, Simon Marchi, Tom Tromey; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1920 bytes --]

Thank you Tom and Simon

From: Tom de Vries <tdevries@suse.de>
Date: Monday, 10 June 2024 at 2:14 PM
To: Simon Marchi <simon.marchi@polymtl.ca>, Tom Tromey <tom@tromey.com>
Cc: gdb-patches@sourceware.org <gdb-patches@sourceware.org>
Subject: [EXTERNAL] Re: [PATCH 3/3] gdb/tui: cleanup includes
On 6/9/24 16:14, Simon Marchi wrote:
>
>
> On 2024-06-08 06:14, Tom de Vries wrote:
>> On 6/3/24 16:57, Tom Tromey wrote:
>>>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
>>>
>>> Simon> Remove includes reported as unused by clangd.  Then, add any includes
>>> Simon> necessary to get rid of errors (includes possibly relying on previous
>>> Simon> includes)..
>>>
>>> Simon> I didn't remove the includes of gdb-safe-ctypes.h, because it appears to
>>> Simon> do some some preprocessor magic, redefining standard macros.  I'm afraid
>>> Simon> that removing these includes could change the behavior unintentionally.
>>>
>>> It's never been super clear when gdb should use the libiberty stuff and
>>> when it should not.
>>>
>>> Approved-By: Tom Tromey <tom@tromey.com>
>>
>> This commit introduced a regression for me:
>> ...
>> Running /data/vries/gdb/src/gdb/testsuite/gdb.python/tui-window.exp ...
>> WARNING: timeout in accept_gdb_output
>> WARNING: timeout in accept_gdb_output
>> FAIL: gdb.python/tui-window.exp: Window was updated
>> ...
>>
>> Filed here (  https://sourceware.org/bugzilla/show_bug.cgi?id=31865  ) with gdb.log attached.
>>
>> Thanks,
>> - Tom
>>
>
> Sorry, I messed up. See the patch here:
>
> https://inbox.sourceware.org/gdb-patches/20240609141034.238889-1-simon.marchi@polymtl.ca/T/#u
>
> I'm currently on leave and won't necessarily be near a computer all day, so if you think this
> patch is ok, feel free to push it on my behalf in order to fix the build as quickly as possible.
>

Thanks for the quick fix, pushed.

- Tom

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-06-12  4:29 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-31  2:54 [PATCH 1/3] gdb/tui: change some macros to functions Simon Marchi
2024-05-31  2:54 ` [PATCH 2/3] gdb: add IWYU export pragams to gdb_curses.h Simon Marchi
2024-06-03 14:56   ` Tom Tromey
2024-06-08  3:07     ` Simon Marchi
2024-05-31  2:54 ` [PATCH 3/3] gdb/tui: cleanup includes Simon Marchi
2024-06-03 14:57   ` Tom Tromey
2024-06-08 10:14     ` Tom de Vries
2024-06-09 14:14       ` Simon Marchi
2024-06-10  8:44         ` Tom de Vries
2024-06-12  4:29           ` Aditya Kamath1
2024-06-09  5:51     ` Aditya Kamath1
2024-06-03 14:55 ` [PATCH 1/3] gdb/tui: change some macros to functions Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).