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

[Doug Evans <dje at google dot com>]

On Mon, Jan 2, 2012 at 11:13 AM, Tom Tromey <tromey@redhat.com> wrote:
> 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.

Sigh.
This is what I get for monkey-see-monkey-do hacking.
You can't trust any part of gdb to be what the powers-that-be find acceptable.
[The code in question is far from rare, and any details are certainly
not protected in a way that imposes or even suggests a proper API.  A
day I continue to wish for btw.]

> How much more invasive is the alternative?

I'd rather keep the hack as one call site at an outer level than embed
it into new_symbol or each die handler or some such.

> 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.

Or I can.
This is one of the things I want to clean up before checking in.

> 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.

I wasn't sure that would be acceptable.

> Doug> + ? FIXME: Hacky, but until things solidify it's not worth much more. ?*/
>
> I think you could safely remove this FIXME.

I like it.  It reminds me Go's mangling needs to change.

> 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.

I'll replace it with a similar comment.

> 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.

I think that's the general consensus.

> 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?

The choice of what encoding to use is, ultimately, a property of the
thing you are printing, not any global state.  Plus Go generally uses
utf8; I wasn't willing to have the Go support change the target
encoding.