Re: [RFA v2 10/24] Remove make_cleanup_restore_current_language

[Simon Marchi <simon.marchi@ericsson.com>]

Hi Tom,

On 2017-07-25 07:20 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.
> 
> ChangeLog
> 2017-07-25  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 scoped_restore_language.
> 	* mi/mi-main.c (mi_cmd_execute): Use scoped_restore_language.
> 	* language.h (scoped_restore_language): New class.
> ---
>  gdb/ChangeLog    | 10 ++++++++++
>  gdb/language.h   | 26 ++++++++++++++++++++++++++
>  gdb/mi/mi-main.c |  6 ++----
>  gdb/parse.c      | 22 +++++++---------------
>  gdb/utils.c      | 22 ----------------------
>  gdb/utils.h      |  2 --
>  6 files changed, 45 insertions(+), 43 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 871b1f0..37971ec 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,15 @@
>  2017-07-25  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 scoped_restore_language.
> +	* mi/mi-main.c (mi_cmd_execute): Use scoped_restore_language.
> +	* language.h (scoped_restore_language): New class.
> +
> +2017-07-25  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 f4852c1..3586526 100644
> --- a/gdb/language.h
> +++ b/gdb/language.h
> @@ -633,4 +633,30 @@ extern const struct language_defn opencl_language_defn;
>  extern const struct language_defn pascal_language_defn;
>  extern const struct language_defn rust_language_defn;
>  
> +/* Save the current language and restore it upon destruction.  */
> +
> +class scoped_restore_language
> +{
> +public:
> +
> +  explicit scoped_restore_language (enum language new_lang)
> +    : m_lang (current_language->la_language)
> +  {
> +    set_language (new_lang);
> +  }

To follow the nomenclature and behavior of other scoped_restore_* , I think this:

1. should be named scoped_restore_current_language
2. should only only save and restore the current language, and not set the new language.

About #2, I think it's clearer to have:

 scoped_restore_current_language ();
 set_language (some_language);

than

 scope_restore_current_language (some_language);

The 2nd leads me to think that the current language will be restored to some_language.

Thanks,

Simon