Re: [RFA 6/8] Use value_freer in dwarf2_evaluate_loc_desc_full

[Pedro Alves <palves@redhat.com>]

On 11/29/2016 05:05 AM, Tom Tromey wrote:
> This changes dwarf2_evaluate_loc_desc_full to use value_freer.
> 
> Note that this function previously called do_cleanup using the same
> cleanup multiple times.  I had thought this was buggy, but re-reading
> make_my_cleanup2 indicates that it is not.  Nevertheless it is
> surprising, and at least one of the calls (the one that is completely
> removed in this patch) seems to have been done under the assumption
> that it would still have some effect.
> 
> 2016-11-28  Tom Tromey  <tom@tromey.com>
> 
> 	* value.h (value_freer::~value_freer): Call release.
> 	(value_freer::release): New method.
> 	* dwarf2loc.c (dwarf2_evaluate_loc_desc_full): Use value_freer.
> ---
>  gdb/ChangeLog   |  6 ++++++
>  gdb/dwarf2loc.c | 27 +++++++++++++--------------
>  gdb/value.h     | 12 +++++++++++-
>  3 files changed, 30 insertions(+), 15 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 6197bfb..cf61306 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,11 @@
>  2016-11-28  Tom Tromey  <tom@tromey.com>
>  
> +	* value.h (value_freer::~value_freer): Call release.
> +	(value_freer::release): New method.
> +	* dwarf2loc.c (dwarf2_evaluate_loc_desc_full): Use value_freer.
> +
> +2016-11-28  Tom Tromey  <tom@tromey.com>
> +
>  	* python/py-value.c (valpy_dereference, valpy_referenced_value)
>  	(valpy_reference_value, valpy_const_value, valpy_get_address)
>  	(valpy_get_dynamic_type, valpy_lazy_string, valpy_do_cast)
> diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
> index 0d8a47c..43c95b8 100644
> --- a/gdb/dwarf2loc.c
> +++ b/gdb/dwarf2loc.c
> @@ -2321,7 +2321,6 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
>  			       LONGEST byte_offset)
>  {
>    struct value *retval;
> -  struct cleanup *value_chain;
>    struct objfile *objfile = dwarf2_per_cu_objfile (per_cu);
>  
>    if (byte_offset < 0)
> @@ -2335,7 +2334,7 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
>    ctx.per_cu = per_cu;
>    ctx.obj_address = 0;
>  
> -  value_chain = make_cleanup_value_free_to_mark (value_mark ());
> +  value_freer free_values;
>  
>    ctx.gdbarch = get_objfile_arch (objfile);
>    ctx.addr_size = dwarf2_per_cu_addr_size (per_cu);
> @@ -2350,7 +2349,7 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
>      {
>        if (ex.error == NOT_AVAILABLE_ERROR)
>  	{
> -	  do_cleanups (value_chain);
> +	  free_values.release ();

"release ()" looks potentially confusing to me, if you're in an
"unique_ptr" mindset, where release means the opposite -- to stop
managing.  Can we call that something else?

Maybe "free", since the class is called value_freer?

Or "reset ()", following the naming used in the standard smart pointers?

The latter could be naturally extended to support

  free_values.reset (value_mark ());

later too, if we need it.

Thanks,
Pedro Alves