Re: [RFA 10/23] Remove make_cleanup_restore_current_language

[Pedro Alves <palves at redhat dot com>]

This is OK, but we may we want to take a closer look at
terminology -- we started with scoped_restore, and have added
other types called scoped_restore_whatever meanwhile.
But here we're calling it "save".  It seems to me like it may be
better in the long run to pick one naming style for save/restore
types, and stick with it.

Thanks,
Pedro Alves

On 05/03/2017 11:46 PM, Tom Tromey wrote:
> This patch replaces make_cleanup_restore_current_language with an RAII
> class that saves the current language, and restores it when the object
> is destroyed.
> 
> 2017-05-02  Tom Tromey  <tom@tromey.com>
> 
> 	* utils.h (make_cleanup_restore_current_language): Remove.
> 	* utils.c (do_restore_current_language)
> 	(make_cleanup_restore_current_language): Remove.
> 	* parse.c (parse_exp_in_context_1)
> 	(parse_expression_with_language): Use save_language.
> 	* mi/mi-main.c (mi_cmd_execute): Use save_language.
> 	* language.h (save_language): New class.
> ---
>  gdb/ChangeLog    | 10 ++++++++++
>  gdb/language.h   | 25 +++++++++++++++++++++++++
>  gdb/mi/mi-main.c |  6 ++----
>  gdb/parse.c      | 22 +++++++---------------
>  gdb/utils.c      | 22 ----------------------
>  gdb/utils.h      |  2 --
>  6 files changed, 44 insertions(+), 43 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 50d8794..1c3b738 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,15 @@
>  2017-05-02  Tom Tromey  <tom@tromey.com>
>  
> +	* utils.h (make_cleanup_restore_current_language): Remove.
> +	* utils.c (do_restore_current_language)
> +	(make_cleanup_restore_current_language): Remove.
> +	* parse.c (parse_exp_in_context_1)
> +	(parse_expression_with_language): Use save_language.
> +	* mi/mi-main.c (mi_cmd_execute): Use save_language.
> +	* language.h (save_language): New class.
> +
> +2017-05-02  Tom Tromey  <tom@tromey.com>
> +
>  	* source.c (get_filename_and_charpos, forward_search_command)
>  	(reverse_search_command): Use fd_closer.
>  	* procfs.c (load_syscalls, proc_get_LDT_entry)
> diff --git a/gdb/language.h b/gdb/language.h
> index 58bcbe8..f543da5 100644
> --- a/gdb/language.h
> +++ b/gdb/language.h
> @@ -609,4 +609,29 @@ void default_get_string (struct value *value, gdb_byte **buffer, int *length,
>  void c_get_string (struct value *value, gdb_byte **buffer, int *length,
>  		   struct type **char_type, const char **charset);
>  
> +/* Save the current language and restore it upon destruction.  */
> +
> +class save_language
> +{
> +public:
> +
> +  explicit save_language (enum language new_lang)
> +    : m_lang (current_language->la_language)
> +  {
> +    set_language (new_lang);
> +  }
> +
> +  ~save_language ()
> +  {
> +    set_language (m_lang);
> +  }
> +
> +  save_language (const save_language &) = delete;
> +  save_language &operator= (const save_language &) = delete;
> +
> +private:
> +
> +  enum language m_lang;
> +};
> +
>  #endif /* defined (LANGUAGE_H) */
> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> index cb68fd6..aa44876 100644
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -2242,11 +2242,9 @@ mi_cmd_execute (struct mi_parse *parse)
>  	error (_("Invalid frame id: %d"), frame);
>      }
>  
> +  gdb::optional<save_language> lang_saver;
>    if (parse->language != language_unknown)
> -    {
> -      make_cleanup_restore_current_language ();
> -      set_language (parse->language);
> -    }
> +    lang_saver.emplace (parse->language);
>  
>    current_context = parse;
>  
> diff --git a/gdb/parse.c b/gdb/parse.c
> index 3dd7075..84fce19 100644
> --- a/gdb/parse.c
> +++ b/gdb/parse.c
> @@ -50,6 +50,7 @@
>  #include "objfiles.h"
>  #include "user-regs.h"
>  #include <algorithm>
> +#include "common/gdb_optional.h"
>  
>  /* Standard set of definitions for printing, dumping, prefixifying,
>   * and evaluating expressions.  */
> @@ -1136,7 +1137,7 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc,
>  			const struct block *block,
>  			int comma, int void_context_p, int *out_subexp)
>  {
> -  struct cleanup *old_chain, *inner_chain;
> +  struct cleanup *old_chain;
>    const struct language_defn *lang = NULL;
>    struct parser_state ps;
>    int subexp;
> @@ -1214,8 +1215,8 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc,
>       to the value matching SELECTED_FRAME as set by get_current_arch.  */
>  
>    initialize_expout (&ps, 10, lang, get_current_arch ());
> -  inner_chain = make_cleanup_restore_current_language ();
> -  set_language (lang->la_language);
> +
> +  save_language lang_saver (lang->la_language);
>  
>    TRY
>      {
> @@ -1250,7 +1251,6 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc,
>    if (expressiondebug)
>      dump_prefix_expression (ps.expout, gdb_stdlog);
>  
> -  do_cleanups (inner_chain);
>    discard_cleanups (old_chain);
>  
>    *stringptr = lexptr;
> @@ -1275,19 +1275,11 @@ parse_expression (const char *string)
>  expression_up
>  parse_expression_with_language (const char *string, enum language lang)
>  {
> -  struct cleanup *old_chain = NULL;
> -
> +  gdb::optional<save_language> lang_saver;
>    if (current_language->la_language != lang)
> -    {
> -      old_chain = make_cleanup_restore_current_language ();
> -      set_language (lang);
> -    }
> -
> -  expression_up expr = parse_expression (string);
> +    lang_saver.emplace (lang);
>  
> -  if (old_chain != NULL)
> -    do_cleanups (old_chain);
> -  return expr;
> +  return parse_expression (string);
>  }
>  
>  /* Parse STRING as an expression.  If parsing ends in the middle of a
> diff --git a/gdb/utils.c b/gdb/utils.c
> index bffce78..1a66896 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -307,28 +307,6 @@ make_cleanup_free_so (struct so_list *so)
>    return make_cleanup (do_free_so, so);
>  }
>  
> -/* Helper for make_cleanup_restore_current_language.  */
> -
> -static void
> -do_restore_current_language (void *p)
> -{
> -  enum language saved_lang = (enum language) (uintptr_t) p;
> -
> -  set_language (saved_lang);
> -}
> -
> -/* Remember the current value of CURRENT_LANGUAGE and make it restored when
> -   the cleanup is run.  */
> -
> -struct cleanup *
> -make_cleanup_restore_current_language (void)
> -{
> -  enum language saved_lang = current_language->la_language;
> -
> -  return make_cleanup (do_restore_current_language,
> -		       (void *) (uintptr_t) saved_lang);
> -}
> -
>  /* Helper function for make_cleanup_clear_parser_state.  */
>  
>  static void
> diff --git a/gdb/utils.h b/gdb/utils.h
> index 7afb3c5..1f01bdd 100644
> --- a/gdb/utils.h
> +++ b/gdb/utils.h
> @@ -91,8 +91,6 @@ extern struct cleanup *make_cleanup_value_free (struct value *);
>  struct so_list;
>  extern struct cleanup *make_cleanup_free_so (struct so_list *so);
>  
> -extern struct cleanup *make_cleanup_restore_current_language (void);
> -
>  /* A deleter for a hash table.  */
>  struct htab_deleter
>  {
>