[RFA,3/5] Use std::string, std::vector in rust-lang.c

  This patche changes some spots in rust-lang.c to use std::string or
std::vector, removing some cleanups.

2016-09-22  Tom Tromey  <tom@tromey.com>

	* rust-lang.c: Include <string> and <vector>.
	(rust_evaluate_funcall): Use std::vector, std::string.
	(rust_evaluate_subexp): Use std::string.
	(rust_lookup_symbol_nonlocal): Use std::string.
---
 gdb/ChangeLog   |  7 +++++++
 gdb/rust-lang.c | 38 ++++++++++++++++----------------------
 2 files changed, 23 insertions(+), 22 deletions(-)
  

On 09/22/2016 06:50 PM, Tom Tromey wrote:
>    int num_args = exp->elts[*pos + 1].longconst;
>    const char *method;
> -  char *name;
> +  std::string name;

While we required declaring variables at the top of the
scope in C, in C++ it no longer makes sense.  Particularly
if the variable's type as a constructor.

So I think we should move the variable declaration to the
initialization line, to avoid default constructing the variable
and then resetting it afterwards, as the compiler may
not be smart enough to elide that.

>  
> -  name = concat (TYPE_TAG_NAME (type), "::", method, (char *) NULL);
> -  make_cleanup (xfree, name);
> +  name = std::string (TYPE_TAG_NAME (type)) + "::" + method;
>  


> -	  char *scopedname = concat (scope, "::", name, (char *) NULL);
> -	  struct cleanup *cleanup = make_cleanup (xfree, scopedname);
> +	  std::string scopedname;
> +
> +	  scopedname = std::string (scope) + "::" + name;

Here too.  Throughout the series, really.

Thanks,
Pedro Alves
  

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> So I think we should move the variable declaration to the
Pedro> initialization line, to avoid default constructing the variable
Pedro> and then resetting it afterwards, as the compiler may
Pedro> not be smart enough to elide that.

I think so too -- I had avoided this on account of earlier objections to
this style.

Tom
  

On 09/22/2016 08:15 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> So I think we should move the variable declaration to the
> Pedro> initialization line, to avoid default constructing the variable
> Pedro> and then resetting it afterwards, as the compiler may
> Pedro> not be smart enough to elide that.
> 
> I think so too -- I had avoided this on account of earlier objections to
> this style.

In C, it'd require C99, while we were C89.  Other than that,
it was just a coding convention.  

But we now have technical reasons for not putting variables
at top of the scope.  There's the efficiency reason, and then
some types may not even have a default constructor, and the
arguments that'd need to be passed to the constructor
might not have been computed at the top of the scope, making
it impossible to declare the variable at the top, unless we'd
open an ugly new scope...

So I think there's no ground for objection.

Thanks,
Pedro Alves
  

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : jeudi 22 septembre 2016 21:25
> À : Tom Tromey
> Cc : gdb-patches@sourceware.org
> Objet : Re: [RFA 3/5] Use std::string, std::vector in rust-lang.c
> 
> On 09/22/2016 08:15 PM, Tom Tromey wrote:
> >>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> >
> > Pedro> So I think we should move the variable declaration to the
> > Pedro> initialization line, to avoid default constructing the
> variable
> > Pedro> and then resetting it afterwards, as the compiler may
> > Pedro> not be smart enough to elide that.
> >
> > I think so too -- I had avoided this on account of earlier objections
> to
> > this style.
> 
> In C, it'd require C99, while we were C89.  Other than that,
> it was just a coding convention.
> 
> But we now have technical reasons for not putting variables
> at top of the scope.  There's the efficiency reason, and then
> some types may not even have a default constructor, and the
> arguments that'd need to be passed to the constructor
> might not have been computed at the top of the scope, making
> it impossible to declare the variable at the top, unless we'd
> open an ugly new scope...

  Just out of curiosity:
how are such variables treated by gcc regarding debug information?

  What is the scope of definition of those variables?
And is it correctly handled by current GDB?

  IIRC (but my knowledge about C++ if far
worse than for plain C.. which itself I only learned to be able to
contribute to GDB...) the reason why we can remove the cleanups is
that the compiler will insert auto--magically
the destructors of those local variables at the end of
their respective scope, is this correct? 
 
> So I think there's no ground for objection.

  Not that I want to raise an objection,
I just wonder if debugging GDB by itself will remain as easy as it was!

Pierre Muller
pascal language support maintainer...
  

On 09/22/2016 08:35 PM, Pierre Muller wrote:

>   Just out of curiosity:
> how are such variables treated by gcc regarding debug information?
> 
>   What is the scope of definition of those variables?
> And is it correctly handled by current GDB?

On my system, they look like as if declared at the top of
the scope.  Up until they're initialized, they'll have garbage
contents, and that's what gdb will show.  But that's no different from
actually declaring a variable at the top of the scope and only
initializing it after.  It's not a big deal.

There's a GNU extension that I think is meant to make it possible
for the debugger to treat uninitialized variables
specially (DW_OP_GNU_uninit), though I don't think we do
much with it.  Not sure gcc emits it, even.

> 
>   IIRC (but my knowledge about C++ if far
> worse than for plain C.. which itself I only learned to be able to
> contribute to GDB...) the reason why we can remove the cleanups is
> that the compiler will insert auto--magically
> the destructors of those local variables at the end of
> their respective scope, is this correct? 

Correct.

>  
>> So I think there's no ground for objection.
> 
>   Not that I want to raise an objection,
> I just wonder if debugging GDB by itself will remain as easy as it was!

Hope so too...

Thanks,
Pedro Alves
  

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> There's a GNU extension that I think is meant to make it possible
Pedro> for the debugger to treat uninitialized variables
Pedro> specially (DW_OP_GNU_uninit), though I don't think we do
Pedro> much with it.

There were never any docs for it :(
I think it probably should have been a piece-terminating operation but
it was never clear if this was how it was actually supposed to work.
I stuck a comment in dwarf_expr_require_composition to this effect back
in the day.

Pedro> Not sure gcc emits it, even.

It does, but I don't know under what conditions.
There's code in dwarf2out.c like:

  if (initialized == VAR_INIT_STATUS_UNINITIALIZED)
    add_loc_descr (&result, new_loc_descr (DW_OP_GNU_uninit, 0, 0));

I don't know when that happens though.

Perhaps this could be cleaned up a bit, say by making DW_OP_GNU_uninit a
piece terminator (I forget the official term here) and by changing gcc
to emit a location list that indicates that the object is uninitialized
until after the constructor is run.  It might be a little friendlier to
users, dunno.

Tom
  

On 09/23/2016 04:49 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> There's a GNU extension that I think is meant to make it possible
> Pedro> for the debugger to treat uninitialized variables
> Pedro> specially (DW_OP_GNU_uninit), though I don't think we do
> Pedro> much with it.
> 
> There were never any docs for it :(

I think all there is the original gcc and gdb patches:

  https://gcc.gnu.org/ml/gcc-patches/2007-05/msg00068.html

I remember seeing these back then, but I was still a gdb noob then.
I don't recall the history of the feature after that.  From a quick look,
it kind of looks like some pieces of the patches are in the respective
trees, but not all hunks.  Maybe they got in, and then mostly reverted.

Thanks,
Pedro Alves

> I think it probably should have been a piece-terminating operation but
> it was never clear if this was how it was actually supposed to work.
> I stuck a comment in dwarf_expr_require_composition to this effect back
> in the day.
> 
> Pedro> Not sure gcc emits it, even.
> 
> It does, but I don't know under what conditions.
> There's code in dwarf2out.c like:
> 
>   if (initialized == VAR_INIT_STATUS_UNINITIALIZED)
>     add_loc_descr (&result, new_loc_descr (DW_OP_GNU_uninit, 0, 0));
> 
> I don't know when that happens though.
> 
> Perhaps this could be cleaned up a bit, say by making DW_OP_GNU_uninit a
> piece terminator (I forget the official term here) and by changing gcc
> to emit a location list that indicates that the object is uninitialized
> until after the constructor is run.  It might be a little friendlier to
> users, dunno.
> 
> Tom
>