* [RFA 0/4] minor interp-related cleanups @ 2018-04-30 5:12 Tom Tromey 2018-04-30 5:12 ` [RFA 3/4] Remove interp_ui_out Tom Tromey ` (4 more replies) 0 siblings, 5 replies; 12+ messages in thread From: Tom Tromey @ 2018-04-30 5:12 UTC (permalink / raw) To: gdb-patches I found a few small things in the interp code that I thought could be improved. This short series is the result. Regression tested by the buildbot. Tom ^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFA 3/4] Remove interp_ui_out 2018-04-30 5:12 [RFA 0/4] minor interp-related cleanups Tom Tromey @ 2018-04-30 5:12 ` Tom Tromey 2018-05-25 18:37 ` Pedro Alves 2018-04-30 5:12 ` [RFA 4/4] Remove interp_name Tom Tromey ` (3 subsequent siblings) 4 siblings, 1 reply; 12+ messages in thread From: Tom Tromey @ 2018-04-30 5:12 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey The function interp_ui_out simply calls the interp_ui_out method. However, if it is passed a NULL interpreter, it first finds the current interpreter. I believe, though, that NULL is never passed here, and I think it's simpler to just remove this function and require callers to be more explicit. ChangeLog 2018-04-29 Tom Tromey <tom@tromey.com> * utils.c (fputs_maybe_filtered): Update. * linespec.c (decode_line_full): Update. * mi/mi-interp.c (mi_on_normal_stop_1, mi_tsv_modified) (mi_print_breakpoint_for_event, mi_solib_loaded) (mi_solib_unloaded, mi_command_param_changed, mi_memory_changed) (mi_user_selected_context_changed): Update. * mi/mi-main.c (mi_execute_command): Update. * cli/cli-script.c (execute_control_command): Update. * python/python.c (execute_gdb_command): Update. * solib.c (info_sharedlibrary_command): Update. * interps.c (interp_ui_out): Remove. * interps.h (interp_ui_out): Remove. --- gdb/ChangeLog | 15 +++++++++++++++ gdb/cli/cli-script.c | 2 +- gdb/interps.c | 12 ------------ gdb/interps.h | 1 - gdb/linespec.c | 2 +- gdb/mi/mi-interp.c | 16 ++++++++-------- gdb/mi/mi-main.c | 2 +- gdb/python/python.c | 2 +- gdb/solib.c | 2 +- gdb/utils.c | 2 +- 10 files changed, 29 insertions(+), 27 deletions(-) diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c index cdfda113a7..717dbe4c20 100644 --- a/gdb/cli/cli-script.c +++ b/gdb/cli/cli-script.c @@ -642,7 +642,7 @@ execute_control_command (struct command_line *cmd) breakpoint commands while running the MI interpreter. */ interp *console = interp_lookup (current_ui, INTERP_CONSOLE); scoped_restore save_uiout - = make_scoped_restore (¤t_uiout, interp_ui_out (console)); + = make_scoped_restore (¤t_uiout, console->interp_ui_out ()); return execute_control_command_1 (cmd); } diff --git a/gdb/interps.c b/gdb/interps.c index 162cd838ec..5884625856 100644 --- a/gdb/interps.c +++ b/gdb/interps.c @@ -251,18 +251,6 @@ set_top_level_interpreter (const char *name) interp_set (interp, true); } -/* Returns the current interpreter. */ - -struct ui_out * -interp_ui_out (struct interp *interp) -{ - struct ui_interp_info *ui_interp = get_current_interp_info (); - - if (interp == NULL) - interp = ui_interp->current_interpreter; - return interp->interp_ui_out (); -} - void current_interp_set_logging (ui_file_up logfile, bool logging_redirect) diff --git a/gdb/interps.h b/gdb/interps.h index 067c9ddefd..694149a6ca 100644 --- a/gdb/interps.h +++ b/gdb/interps.h @@ -98,7 +98,6 @@ extern struct interp *interp_lookup (struct ui *ui, const char *name); interpreter fails to initialize. */ extern void set_top_level_interpreter (const char *name); -extern struct ui_out *interp_ui_out (struct interp *interp); extern const char *interp_name (struct interp *interp); /* Temporarily set the current interpreter, and reset it on diff --git a/gdb/linespec.c b/gdb/linespec.c index 194d50cb3a..55ab46e0ec 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -3293,7 +3293,7 @@ decode_line_full (const struct event_location *location, int flags, if (select_mode == NULL) { - if (interp_ui_out (top_level_interpreter ())->is_mi_like_p ()) + if (top_level_interpreter ()->interp_ui_out ()->is_mi_like_p ()) select_mode = multiple_symbols_all; else select_mode = multiple_symbols_select_mode (); diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c index 7536817f43..99ce1eb5d6 100644 --- a/gdb/mi/mi-interp.c +++ b/gdb/mi/mi-interp.c @@ -625,7 +625,7 @@ mi_on_normal_stop_1 (struct bpstats *bs, int print_frame) /* Since this can be called when CLI command is executing, using cli interpreter, be sure to use MI uiout for output, not the current one. */ - struct ui_out *mi_uiout = interp_ui_out (top_level_interpreter ()); + struct ui_out *mi_uiout = top_level_interpreter ()->interp_ui_out (); struct mi_interp *mi = (struct mi_interp *) top_level_interpreter (); if (print_frame) @@ -802,7 +802,7 @@ mi_tsv_modified (const struct trace_state_variable *tsv) if (mi == NULL) continue; - mi_uiout = interp_ui_out (top_level_interpreter ()); + mi_uiout = top_level_interpreter ()->interp_ui_out (); target_terminal::scoped_restore_terminal_state term_state; target_terminal::ours_for_output (); @@ -829,7 +829,7 @@ mi_tsv_modified (const struct trace_state_variable *tsv) static void mi_print_breakpoint_for_event (struct mi_interp *mi, breakpoint *bp) { - ui_out *mi_uiout = interp_ui_out (mi); + ui_out *mi_uiout = mi->interp_ui_out (); /* We want the output from print_breakpoint to go to mi->event_channel. One approach would be to just call @@ -1090,7 +1090,7 @@ mi_solib_loaded (struct so_list *solib) if (mi == NULL) continue; - uiout = interp_ui_out (top_level_interpreter ()); + uiout = top_level_interpreter ()->interp_ui_out (); target_terminal::scoped_restore_terminal_state term_state; target_terminal::ours_for_output (); @@ -1118,7 +1118,7 @@ mi_solib_unloaded (struct so_list *solib) if (mi == NULL) continue; - uiout = interp_ui_out (top_level_interpreter ()); + uiout = top_level_interpreter ()->interp_ui_out (); target_terminal::scoped_restore_terminal_state term_state; target_terminal::ours_for_output (); @@ -1157,7 +1157,7 @@ mi_command_param_changed (const char *param, const char *value) if (mi == NULL) continue; - mi_uiout = interp_ui_out (top_level_interpreter ()); + mi_uiout = top_level_interpreter ()->interp_ui_out (); target_terminal::scoped_restore_terminal_state term_state; target_terminal::ours_for_output (); @@ -1193,7 +1193,7 @@ mi_memory_changed (struct inferior *inferior, CORE_ADDR memaddr, if (mi == NULL) continue; - mi_uiout = interp_ui_out (top_level_interpreter ()); + mi_uiout = top_level_interpreter ()->interp_ui_out (); target_terminal::scoped_restore_terminal_state term_state; target_terminal::ours_for_output (); @@ -1246,7 +1246,7 @@ mi_user_selected_context_changed (user_selected_what selection) if (mi == NULL) continue; - mi_uiout = interp_ui_out (top_level_interpreter ()); + mi_uiout = top_level_interpreter ()->interp_ui_out (); mi_uiout->redirect (mi->event_channel); ui_out_redirect_pop redirect_popper (mi_uiout); diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index d7354606b5..980af477d3 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -1996,7 +1996,7 @@ mi_execute_command (const char *cmd, int from_tty) if (/* The notifications are only output when the top-level interpreter (specified on the command line) is MI. */ - interp_ui_out (top_level_interpreter ())->is_mi_like_p () + top_level_interpreter ()->interp_ui_out ()->is_mi_like_p () /* Don't try report anything if there are no threads -- the program is dead. */ && thread_count () != 0 diff --git a/gdb/python/python.c b/gdb/python/python.c index 9eae8a1aef..27e6d9a577 100644 --- a/gdb/python/python.c +++ b/gdb/python/python.c @@ -588,7 +588,7 @@ execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw) /* Use the console interpreter uiout to have the same print format for console or MI. */ interp = interp_lookup (current_ui, "console"); - current_uiout = interp_ui_out (interp); + current_uiout = interp->interp_ui_out (); scoped_restore preventer = prevent_dont_repeat (); if (to_string) diff --git a/gdb/solib.c b/gdb/solib.c index 5011bd7df1..b28ebc628c 100644 --- a/gdb/solib.c +++ b/gdb/solib.c @@ -1093,7 +1093,7 @@ info_sharedlibrary_command (const char *pattern, int from_tty) uiout->field_skip ("to"); } - if (! interp_ui_out (top_level_interpreter ())->is_mi_like_p () + if (! top_level_interpreter ()->interp_ui_out ()->is_mi_like_p () && so->symbols_loaded && !objfile_has_symbols (so->objfile)) { diff --git a/gdb/utils.c b/gdb/utils.c index b957b0dc5c..f6f4b97536 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -1682,7 +1682,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream, || batch_flag || (lines_per_page == UINT_MAX && chars_per_line == UINT_MAX) || top_level_interpreter () == NULL - || interp_ui_out (top_level_interpreter ())->is_mi_like_p ()) + || top_level_interpreter ()->interp_ui_out ()->is_mi_like_p ()) { fputs_unfiltered (linebuffer, stream); return; -- 2.13.6 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA 3/4] Remove interp_ui_out 2018-04-30 5:12 ` [RFA 3/4] Remove interp_ui_out Tom Tromey @ 2018-05-25 18:37 ` Pedro Alves 0 siblings, 0 replies; 12+ messages in thread From: Pedro Alves @ 2018-05-25 18:37 UTC (permalink / raw) To: Tom Tromey, gdb-patches On 04/30/2018 06:12 AM, Tom Tromey wrote: > The function interp_ui_out simply calls the interp_ui_out method. > However, if it is passed a NULL interpreter, it first finds the > current interpreter. I believe, though, that NULL is never passed > here, and I think it's simpler to just remove this function and > require callers to be more explicit. OK. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFA 4/4] Remove interp_name 2018-04-30 5:12 [RFA 0/4] minor interp-related cleanups Tom Tromey 2018-04-30 5:12 ` [RFA 3/4] Remove interp_ui_out Tom Tromey @ 2018-04-30 5:12 ` Tom Tromey 2018-05-25 18:41 ` Pedro Alves 2018-04-30 5:12 ` [RFA 2/4] Change the as_*_interp functions to use dynamic_cast Tom Tromey ` (2 subsequent siblings) 4 siblings, 1 reply; 12+ messages in thread From: Tom Tromey @ 2018-04-30 5:12 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey This removes the interp_name function. It is only used a few spots -- one of which was only calling it on "this". It's simpler to remove it; and should class interp become opaque in the future, it will be just as easy to update the two remaining spots to use an accessor. ChangeLog 2018-04-29 Tom Tromey <tom@tromey.com> * interps.c (interp_name): Remove. * mi/mi-interp.c (mi_interp::init): Update. * interps.h (interp_name): Remove. (~scoped_restore_interp): Update. * tui/tui.c (tui_enable): Update. --- gdb/ChangeLog | 8 ++++++++ gdb/interps.c | 8 -------- gdb/interps.h | 4 +--- gdb/mi/mi-interp.c | 2 -- gdb/tui/tui.c | 2 +- 5 files changed, 10 insertions(+), 14 deletions(-) diff --git a/gdb/interps.c b/gdb/interps.c index 5884625856..8ec9744fdf 100644 --- a/gdb/interps.c +++ b/gdb/interps.c @@ -274,14 +274,6 @@ scoped_restore_interp::set_interp (const char *name) return old_interp; } -/* Returns the interpreter's name. */ - -const char * -interp_name (struct interp *interp) -{ - return interp->name; -} - /* Returns true if the current interp is the passed in name. */ int current_interp_named_p (const char *interp_name) diff --git a/gdb/interps.h b/gdb/interps.h index 694149a6ca..a689be5625 100644 --- a/gdb/interps.h +++ b/gdb/interps.h @@ -98,8 +98,6 @@ extern struct interp *interp_lookup (struct ui *ui, const char *name); interpreter fails to initialize. */ extern void set_top_level_interpreter (const char *name); -extern const char *interp_name (struct interp *interp); - /* Temporarily set the current interpreter, and reset it on destruction. */ class scoped_restore_interp @@ -113,7 +111,7 @@ public: ~scoped_restore_interp () { - set_interp (interp_name (m_interp)); + set_interp (m_interp->name); } scoped_restore_interp (const scoped_restore_interp &) = delete; diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c index 99ce1eb5d6..e52d797973 100644 --- a/gdb/mi/mi-interp.c +++ b/gdb/mi/mi-interp.c @@ -113,7 +113,6 @@ void mi_interp::init (bool top_level) { mi_interp *mi = this; - const char *name; int mi_version; /* Store the current output channel, so that we can create a console @@ -129,7 +128,6 @@ mi_interp::init (bool top_level) mi->targ = new mi_console_file (mi->raw_stdout, "@", '"'); mi->event_channel = new mi_console_file (mi->raw_stdout, "=", 0); - name = interp_name (this); /* INTERP_MI selects the most recent released version. "mi2" was released as part of GDB 6.0. */ if (strcmp (name, INTERP_MI) == 0) diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c index 7943a61676..9e2520b4fc 100644 --- a/gdb/tui/tui.c +++ b/gdb/tui/tui.c @@ -415,7 +415,7 @@ tui_enable (void) /* If the top level interpreter is not the console/tui (e.g., MI), enabling curses will certainly lose. */ - interp = interp_name (top_level_interpreter ()); + interp = top_level_interpreter ()->name; if (strcmp (interp, INTERP_TUI) != 0) error (_("Cannot enable the TUI when the interpreter is '%s'"), interp); -- 2.13.6 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA 4/4] Remove interp_name 2018-04-30 5:12 ` [RFA 4/4] Remove interp_name Tom Tromey @ 2018-05-25 18:41 ` Pedro Alves 2018-05-25 18:47 ` Tom Tromey 0 siblings, 1 reply; 12+ messages in thread From: Pedro Alves @ 2018-05-25 18:41 UTC (permalink / raw) To: Tom Tromey, gdb-patches On 04/30/2018 06:12 AM, Tom Tromey wrote: > This removes the interp_name function. It is only used a few spots -- > one of which was only calling it on "this". It's simpler to remove > it; and should class interp become opaque in the future, it will be > just as easy to update the two remaining spots to use an accessor. Yeah, IIRC, it used to be opaque until I C++ified it. Not seeing a need to go back. > > ChangeLog > 2018-04-29 Tom Tromey <tom@tromey.com> > > * interps.c (interp_name): Remove. > * mi/mi-interp.c (mi_interp::init): Update. > * interps.h (interp_name): Remove. > (~scoped_restore_interp): Update. > * tui/tui.c (tui_enable): Update. > diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c > index 7943a61676..9e2520b4fc 100644 > --- a/gdb/tui/tui.c > +++ b/gdb/tui/tui.c > @@ -415,7 +415,7 @@ tui_enable (void) > > /* If the top level interpreter is not the console/tui (e.g., > MI), enabling curses will certainly lose. */ > - interp = interp_name (top_level_interpreter ()); > + interp = top_level_interpreter ()->name; > if (strcmp (interp, INTERP_TUI) != 0) > error (_("Cannot enable the TUI when the interpreter is '%s'"), interp); Not sure. The name is supposedly read-only, which would suggest renaming the field to m_name and adding a name() getter. WDYT? Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA 4/4] Remove interp_name 2018-05-25 18:41 ` Pedro Alves @ 2018-05-25 18:47 ` Tom Tromey 2018-05-25 19:36 ` Pedro Alves 0 siblings, 1 reply; 12+ messages in thread From: Tom Tromey @ 2018-05-25 18:47 UTC (permalink / raw) To: Pedro Alves; +Cc: Tom Tromey, gdb-patches >>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes: Pedro> Not sure. The name is supposedly read-only, which would Pedro> suggest renaming the field to m_name and adding a name() Pedro> getter. WDYT? How about the appended as a follow-up patch to this series? Tom commit d525a99be1b02dda6c69007e31dd06f276378aea Author: Tom Tromey <tom@tromey.com> Date: Fri May 25 12:39:51 2018 -0600 Add "name" method to class interp In a review Pedro pointed out that interp::name is intended to be read-only, and so an accessor would be a better fit. This patch renames the field and adds a "name" method that is used instead. ChangeLog 2018-05-25 Tom Tromey <tom@tromey.com> * tui/tui.c (tui_enable): Update. * mi/mi-interp.c (mi_interp::init): Update. * interps.h (class interp) <name>: New method. <m_name>: Rename from name. (~scoped_restore_interp): Update. * interps.c (interp::interp): Update. (interp_add, interp_set, interp_lookup_existing) (current_interp_named_p): Update. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 0bf350c255..a01f60c3b0 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,14 @@ +2018-05-25 Tom Tromey <tom@tromey.com> + + * tui/tui.c (tui_enable): Update. + * mi/mi-interp.c (mi_interp::init): Update. + * interps.h (class interp) <name>: New method. + <m_name>: Rename from name. + (~scoped_restore_interp): Update. + * interps.c (interp::interp): Update. + (interp_add, interp_set, interp_lookup_existing) + (current_interp_named_p): Update. + 2018-05-25 Tom Tromey <tom@tromey.com> * interps.c (interp_name): Remove. diff --git a/gdb/interps.c b/gdb/interps.c index 8ec9744fdf..789ae86946 100644 --- a/gdb/interps.c +++ b/gdb/interps.c @@ -78,8 +78,8 @@ static struct interp *interp_lookup_existing (struct ui *ui, const char *name); interp::interp (const char *name) + : m_name (xstrdup (name)) { - this->name = xstrdup (name); this->inited = false; } @@ -129,7 +129,7 @@ interp_add (struct ui *ui, struct interp *interp) { struct ui_interp_info *ui_interp = get_interp_info (ui); - gdb_assert (interp_lookup_existing (ui, interp->name) == NULL); + gdb_assert (interp_lookup_existing (ui, interp->name ()) == NULL); interp->next = ui_interp->interp_list; ui_interp->interp_list = interp; @@ -170,11 +170,11 @@ interp_set (struct interp *interp, bool top_level) /* We use interpreter_p for the "set interpreter" variable, so we need to make sure we have a malloc'ed copy for the set command to free. */ if (interpreter_p != NULL - && strcmp (interp->name, interpreter_p) != 0) + && strcmp (interp->name (), interpreter_p) != 0) { xfree (interpreter_p); - interpreter_p = xstrdup (interp->name); + interpreter_p = xstrdup (interp->name ()); } /* Run the init proc. */ @@ -206,7 +206,7 @@ interp_lookup_existing (struct ui *ui, const char *name) interp != NULL; interp = interp->next) { - if (strcmp (interp->name, name) == 0) + if (strcmp (interp->name (), name) == 0) return interp; } @@ -282,7 +282,7 @@ current_interp_named_p (const char *interp_name) struct interp *interp = ui_interp->current_interpreter; if (interp != NULL) - return (strcmp (interp->name, interp_name) == 0); + return (strcmp (interp->name (), interp_name) == 0); return 0; } diff --git a/gdb/interps.h b/gdb/interps.h index a689be5625..74c9a80918 100644 --- a/gdb/interps.h +++ b/gdb/interps.h @@ -74,8 +74,13 @@ public: virtual bool supports_command_editing () { return false; } + const char *name () const + { + return m_name; + } + /* This is the name in "-i=" and "set interpreter". */ - const char *name; + const char *m_name; /* Interpreters are stored in a linked list, this is the next one... */ @@ -111,7 +116,7 @@ public: ~scoped_restore_interp () { - set_interp (m_interp->name); + set_interp (m_interp->name ()); } scoped_restore_interp (const scoped_restore_interp &) = delete; diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c index e52d797973..ebc899f631 100644 --- a/gdb/mi/mi-interp.c +++ b/gdb/mi/mi-interp.c @@ -130,13 +130,13 @@ mi_interp::init (bool top_level) /* INTERP_MI selects the most recent released version. "mi2" was released as part of GDB 6.0. */ - if (strcmp (name, INTERP_MI) == 0) + if (strcmp (name (), INTERP_MI) == 0) mi_version = 2; - else if (strcmp (name, INTERP_MI1) == 0) + else if (strcmp (name (), INTERP_MI1) == 0) mi_version = 1; - else if (strcmp (name, INTERP_MI2) == 0) + else if (strcmp (name (), INTERP_MI2) == 0) mi_version = 2; - else if (strcmp (name, INTERP_MI3) == 0) + else if (strcmp (name (), INTERP_MI3) == 0) mi_version = 3; else gdb_assert_not_reached ("unhandled MI version"); diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c index 9e2520b4fc..75a9ced619 100644 --- a/gdb/tui/tui.c +++ b/gdb/tui/tui.c @@ -415,7 +415,7 @@ tui_enable (void) /* If the top level interpreter is not the console/tui (e.g., MI), enabling curses will certainly lose. */ - interp = top_level_interpreter ()->name; + interp = top_level_interpreter ()->name (); if (strcmp (interp, INTERP_TUI) != 0) error (_("Cannot enable the TUI when the interpreter is '%s'"), interp); ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA 4/4] Remove interp_name 2018-05-25 18:47 ` Tom Tromey @ 2018-05-25 19:36 ` Pedro Alves 0 siblings, 0 replies; 12+ messages in thread From: Pedro Alves @ 2018-05-25 19:36 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On 05/25/2018 07:41 PM, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes: > > Pedro> Not sure. The name is supposedly read-only, which would > Pedro> suggest renaming the field to m_name and adding a name() > Pedro> getter. WDYT? > > How about the appended as a follow-up patch to this series? Thanks, looks good. Pedro Alves ^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFA 2/4] Change the as_*_interp functions to use dynamic_cast 2018-04-30 5:12 [RFA 0/4] minor interp-related cleanups Tom Tromey 2018-04-30 5:12 ` [RFA 3/4] Remove interp_ui_out Tom Tromey 2018-04-30 5:12 ` [RFA 4/4] Remove interp_name Tom Tromey @ 2018-04-30 5:12 ` Tom Tromey 2018-05-25 18:12 ` Pedro Alves 2018-04-30 5:12 ` [RFA 1/4] Use scoped_restore in a couple of interp-related places Tom Tromey 2018-05-25 17:34 ` [RFA 0/4] minor interp-related cleanups Tom Tromey 4 siblings, 1 reply; 12+ messages in thread From: Tom Tromey @ 2018-04-30 5:12 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey This changes the various as_*_interp functions to be implemented using dynamic_cast. I believe this is a small improvement, because it is more typesafe -- the C++ runtime does the type-checking for us. ChangeLog 2018-04-29 Tom Tromey <tom@tromey.com> * tui/tui-interp.c (as_tui_interp): Use dynamic_cast. * mi/mi-interp.c (as_mi_interp): Use dynamic_cast. * cli/cli-interp.c (as_cli_interp): Use dynamic_cast. --- gdb/ChangeLog | 6 ++++++ gdb/cli/cli-interp.c | 4 +--- gdb/mi/mi-interp.c | 4 +--- gdb/tui/tui-interp.c | 4 +--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/gdb/cli/cli-interp.c b/gdb/cli/cli-interp.c index 0663301c19..e12d45b4b7 100644 --- a/gdb/cli/cli-interp.c +++ b/gdb/cli/cli-interp.c @@ -73,9 +73,7 @@ struct cli_suppress_notification cli_suppress_notification = static struct cli_interp * as_cli_interp (struct interp *interp) { - if (strcmp (interp_name (interp), INTERP_CONSOLE) == 0) - return (struct cli_interp *) interp; - return NULL; + return dynamic_cast<cli_interp *> (interp); } /* Longjmp-safe wrapper for "execute_command". */ diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c index 8bfb1298a5..7536817f43 100644 --- a/gdb/mi/mi-interp.c +++ b/gdb/mi/mi-interp.c @@ -106,9 +106,7 @@ display_mi_prompt (struct mi_interp *mi) static struct mi_interp * as_mi_interp (struct interp *interp) { - if (interp_ui_out (interp)->is_mi_like_p ()) - return (struct mi_interp *) interp; - return NULL; + return dynamic_cast<mi_interp *> (interp); } void diff --git a/gdb/tui/tui-interp.c b/gdb/tui/tui-interp.c index cf32c905c5..aa5e328ded 100644 --- a/gdb/tui/tui-interp.c +++ b/gdb/tui/tui-interp.c @@ -58,9 +58,7 @@ public: static tui_interp * as_tui_interp (struct interp *interp) { - if (strcmp (interp_name (interp), INTERP_TUI) == 0) - return (tui_interp *) interp; - return NULL; + return dynamic_cast<tui_interp *> (interp); } /* Cleanup the tui before exiting. */ -- 2.13.6 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA 2/4] Change the as_*_interp functions to use dynamic_cast 2018-04-30 5:12 ` [RFA 2/4] Change the as_*_interp functions to use dynamic_cast Tom Tromey @ 2018-05-25 18:12 ` Pedro Alves 0 siblings, 0 replies; 12+ messages in thread From: Pedro Alves @ 2018-05-25 18:12 UTC (permalink / raw) To: Tom Tromey, gdb-patches On 04/30/2018 06:12 AM, Tom Tromey wrote: > This changes the various as_*_interp functions to be implemented using > dynamic_cast. I believe this is a small improvement, because it is > more typesafe -- the C++ runtime does the type-checking for us. OK. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFA 1/4] Use scoped_restore in a couple of interp-related places 2018-04-30 5:12 [RFA 0/4] minor interp-related cleanups Tom Tromey ` (2 preceding siblings ...) 2018-04-30 5:12 ` [RFA 2/4] Change the as_*_interp functions to use dynamic_cast Tom Tromey @ 2018-04-30 5:12 ` Tom Tromey 2018-05-25 18:06 ` Pedro Alves 2018-05-25 17:34 ` [RFA 0/4] minor interp-related cleanups Tom Tromey 4 siblings, 1 reply; 12+ messages in thread From: Tom Tromey @ 2018-04-30 5:12 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey While looking through the "interp" code I found a couple of spots that could use scoped_restore. ChangeLog 2018-04-29 Tom Tromey <tom@tromey.com> * cli/cli-interp.c (safe_execute_command): Ues scoped_restore. * interps.c (interp_exec): Use scoped_restore. --- gdb/ChangeLog | 5 +++++ gdb/cli/cli-interp.c | 8 ++------ gdb/interps.c | 13 +++---------- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/gdb/cli/cli-interp.c b/gdb/cli/cli-interp.c index bbee809880..0663301c19 100644 --- a/gdb/cli/cli-interp.c +++ b/gdb/cli/cli-interp.c @@ -356,11 +356,10 @@ safe_execute_command (struct ui_out *command_uiout, const char *command, int from_tty) { struct gdb_exception e = exception_none; - struct ui_out *saved_uiout; /* Save and override the global ``struct ui_out'' builder. */ - saved_uiout = current_uiout; - current_uiout = command_uiout; + scoped_restore saved_uiout = make_scoped_restore (¤t_uiout, + command_uiout); TRY { @@ -372,9 +371,6 @@ safe_execute_command (struct ui_out *command_uiout, const char *command, } END_CATCH - /* Restore the global builder. */ - current_uiout = saved_uiout; - /* FIXME: cagney/2005-01-13: This shouldn't be needed. Instead the caller should print the exception. */ exception_print (gdb_stderr, e); diff --git a/gdb/interps.c b/gdb/interps.c index 61db7f4bdf..162cd838ec 100644 --- a/gdb/interps.c +++ b/gdb/interps.c @@ -354,18 +354,11 @@ interp_exec (struct interp *interp, const char *command_str) { struct ui_interp_info *ui_interp = get_current_interp_info (); - struct gdb_exception ex; - struct interp *save_command_interp; - /* See `command_interp' for why we do this. */ - save_command_interp = ui_interp->command_interpreter; - ui_interp->command_interpreter = interp; - - ex = interp->exec (command_str); - - ui_interp->command_interpreter = save_command_interp; + scoped_restore save_command_interp + = make_scoped_restore (&ui_interp->command_interpreter, interp); - return ex; + return interp->exec (command_str); } /* A convenience routine that nulls out all the common command hooks. -- 2.13.6 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA 1/4] Use scoped_restore in a couple of interp-related places 2018-04-30 5:12 ` [RFA 1/4] Use scoped_restore in a couple of interp-related places Tom Tromey @ 2018-05-25 18:06 ` Pedro Alves 0 siblings, 0 replies; 12+ messages in thread From: Pedro Alves @ 2018-05-25 18:06 UTC (permalink / raw) To: Tom Tromey, gdb-patches On 04/30/2018 06:12 AM, Tom Tromey wrote: > While looking through the "interp" code I found a couple of spots that > could use scoped_restore. > > ChangeLog > 2018-04-29 Tom Tromey <tom@tromey.com> > > * cli/cli-interp.c (safe_execute_command): Ues scoped_restore. Typo: "Ues" -> "Use". > @@ -372,9 +371,6 @@ safe_execute_command (struct ui_out *command_uiout, const char *command, > } > END_CATCH > > - /* Restore the global builder. */ > - current_uiout = saved_uiout; > - I was a little worried about whether exception_print could use current_uiout, but it seems not. So OK. > /* FIXME: cagney/2005-01-13: This shouldn't be needed. Instead the > caller should print the exception. */ > exception_print (gdb_stderr, e); Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFA 0/4] minor interp-related cleanups 2018-04-30 5:12 [RFA 0/4] minor interp-related cleanups Tom Tromey ` (3 preceding siblings ...) 2018-04-30 5:12 ` [RFA 1/4] Use scoped_restore in a couple of interp-related places Tom Tromey @ 2018-05-25 17:34 ` Tom Tromey 4 siblings, 0 replies; 12+ messages in thread From: Tom Tromey @ 2018-05-25 17:34 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches >>>>> "Tom" == Tom Tromey <tom@tromey.com> writes: Tom> I found a few small things in the interp code that I thought could be Tom> improved. This short series is the result. Tom> Regression tested by the buildbot. Ping. Tom ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-05-25 18:47 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-30 5:12 [RFA 0/4] minor interp-related cleanups Tom Tromey 2018-04-30 5:12 ` [RFA 3/4] Remove interp_ui_out Tom Tromey 2018-05-25 18:37 ` Pedro Alves 2018-04-30 5:12 ` [RFA 4/4] Remove interp_name Tom Tromey 2018-05-25 18:41 ` Pedro Alves 2018-05-25 18:47 ` Tom Tromey 2018-05-25 19:36 ` Pedro Alves 2018-04-30 5:12 ` [RFA 2/4] Change the as_*_interp functions to use dynamic_cast Tom Tromey 2018-05-25 18:12 ` Pedro Alves 2018-04-30 5:12 ` [RFA 1/4] Use scoped_restore in a couple of interp-related places Tom Tromey 2018-05-25 18:06 ` Pedro Alves 2018-05-25 17:34 ` [RFA 0/4] minor interp-related cleanups 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).