Re: [RFC] Initial pass at supporting the Go language

[Tom Tromey <tromey@redhat.com>]

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> This patch is a first pass at supporting the Go language.
Doug> There's still lots to do, but this is a start.

Nice work.

Doug> I have a few things I'd like to clean up before checking in
Doug> but I don't plan on removing all FIXMEs.
Doug> And I still need to document Go specific features.

A few notes below.

Doug> +/* Go objects should be embedded in a DW_TAG_module DIE,
Doug> +   and it's not clear if/how imported objects will appear.
Doug> +   To keep Go support simple until that's worked out,
Doug> +   go back through what we've read and create something usable.
Doug> +   We could do this while processing each DIE, and feels kinda cleaner,
Doug> +   but that way is more invasive.
Doug> +   This is to, for example, allow the user to type "p var" or "b main"
Doug> +   without having to specify the package name, and allow lookups
Doug> +   of module.object to work in contexts that use the expression
Doug> +   parser.  */

I think this over-exposes some buildsym details to dwarf2read.
How much more invasive is the alternative?

Doug> +	 And if not, it should be clearly documented why not.
Doug> +	 OTOH, why are we demangling at all here?
Doug> +	 new_symbol_full assumes we return the mangled name.
Doug> +	 I realize things are changing in this area, I just forget how.  */
Doug> +      if (cu->language == language_go)
Doug> +	{
Doug> +#if 0
Doug> +	  demangled = cu->language_defn->la_demangle (mangled, 0);
Doug> +#else
Doug> +	  /* This is a lie, but we already lie to the caller new_symbol_full.
Doug> +	     This just undoes that lie until things are cleaned up.  */
Doug> +	  demangled = NULL;
Doug> +#endif

I've CC'd Keith to see if he can clear this up.

Doug> +/* FIXME: IWBN to use c-exp.y's parse_number if we could.  */

You could export it as c_parse_number or something like that, I suppose.

Doug> +   FIXME: Hacky, but until things solidify it's not worth much more.  */

I think you could safely remove this FIXME.

Doug> +    /*{"->", RIGHT_ARROW, BINOP_END}, Doesn't exist in Go.  */

Doug> +#if 0 /* -> doesn't exist in Go.  */
Doug> +	if (in_parse_field && tokentab2[i].token == RIGHT_ARROW)
Doug> +	  last_was_structop = 1;
Doug> +#endif

I think you could zap this dead code.

Doug> +  /* TODO(dje): The encapsulation of what a pointer is belongs in value.c.
Doug> +     I.e. If there's going to be unpack_pointer, there should be
Doug> +     unpack_value_field_as_pointer.  Do this until we can get
Doug> +     unpack_value_field_as_pointer.  */
Doug> +  LONGEST addr;

Eventually I want us to get rid of val_print entirely and only have
value_print.  Then this won't be a problem; since you will just use the
value API to access fields.

Doug> +  /* TODO(dje): Perhaps we should pass "UTF8" for ENCODING.
Doug> +     The target encoding is a global switch.
Doug> +     Either choice is problematic.  */
Doug> +  i = val_print_string (elt_type, NULL, addr, length, stream, options);

What is the problem here?

Tom