Re: [PATCH 6/8] Add support for the Rust language

[Tom Tromey <tom at tromey dot com>]

>> +  /* Rust does not currently emit DW_LANG_Rust or DW_LANG_Rust_old.

Pedro> "Currently" is a moving target.  Could you replace it with some
Pedro> version number or some such?

Yeah, good idea.  Eli suggested something similar in the docs as well.

It turns out that the patch to drop DW_LANG_Rust didn't go in yet, since
it causes an assertion failure in LLVM.  This will probably shake out
before the copyright assignments are done anyway.

>> +#define	RUSTDEBUG 1		/* Default to debug support */

Pedro> Is this referenced anywhere, or a leftoever from when this used
Pedro> the bison prefix support?

Just a leftover.

>> +  /* We treat this differently than Ada.  */

Pedro> What does "differently" mean?  

I removed this comment, but basically Rust reuses the OP_AGGREGATE name,
but not the layout in the expression structure.  This seems weird, but
it was handy, and it's already done elsewhere in gdb.

Pedro> Similar to the Fortran opcode, did you think about promoting
Pedro> OP_AGGREGATE out of ada-operator.def ?

Nope!  There's no big difference between std-operator.def and
ada-operator.def now...  just maybe whether op_name_standard handles the
operator or not.

>> +		if (TYPE_CODE (type) == TYPE_CODE_NAMESPACE)
>> +		  goto got_ns;

Pedro> This goto will probably need to go away with C++-ification.
Pedro> Maybe the got_ns label label could be a helper function, called here,
Pedro> and where it is currently defined.  However, other .y files use
Pedro> gotos as well, so guess it shouldn't be a requirement.

I rearranged this a bit to avoid the goto.

>> +
>> +/* The parser error handler.  */
>> +
>> +void
>> +rustyyerror (char *msg)
>> +{
>> +  const char *where = prev_lexptr ? prev_lexptr : lexptr;
>> +  error (_("A %s in expression, near `%s'."), (msg ? msg : "error"), where);

Pedro> _("error"), I suppose.  I note this expands to the non-grammatical "A error".

Pedro> Maybe reword to avoid it?  Maybe drop the "A" ?

I dropped the "A".  I actually copied this function from c-exp.y.  Maybe
this is another wart -- every .y has to implement this function but it
seems to me that there's no great reason for it.

>> +static struct disr_info
>> +rust_get_disr_info (struct type *type, const gdb_byte *valaddr,

Pedro> Did you mean to name these discr_info and rust_get_discr_info ?
Pedro> I mean, the missing 'c'.

Manish wrote this part -- but not to blame him, this actually mirrors
what is done in the Rust compiler.  I don't know why that abbreviation
was chosen by Rust.  However, it is visible in the debuginfo, as enum
discriminants can be named "RUST$ENUM$DISR".

Tom