This is the mail archive of the
mailing list for the GDB project.
Re: [PATCH 6/8] Add support for the Rust language
- From: Tom Tromey <tom at tromey dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: Tom Tromey <tom at tromey dot com>, gdb-patches at sourceware dot org
- Date: Wed, 27 Apr 2016 12:27:13 -0600
- Subject: Re: [PATCH 6/8] Add support for the Rust language
- Authentication-results: sourceware.org; auth=none
- References: <1461725371-17620-1-git-send-email-tom at tromey dot com> <1461725371-17620-7-git-send-email-tom at tromey dot com> <5720A5E4 dot 2020603 at redhat 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. */
>> +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".