Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Tom Tromey <tom@tromey.com>
To: Pedro Alves <palves@redhat.com>
Cc: Tom Tromey <tom@tromey.com>,  gdb-patches@sourceware.org
Subject: Re: [PATCH 0/8] Add Rust language support
Date: Wed, 27 Apr 2016 16:22:00 -0000	[thread overview]
Message-ID: <87inz3c98x.fsf@tromey.com> (raw)
In-Reply-To: <5720A6B6.8010505@redhat.com> (Pedro Alves's message of "Wed, 27	Apr 2016 12:47:02 +0100")

Tom> If anybody cares, I have a list of the ugly bits in gdb I encountered
Tom> while writing this series.  I think it's all generally well known
Tom> though.

Pedro> I'd be curious.

First, let me say a few words about the good parts.  It's easy to add a
new language.  You pretty much start with struct language_defn and work
out -- there are only a couple of small holes with this plan (see
below).  (FWIW I'm planning to update the wiki with my experiences.)

One thing I especially like is that it's possible to do the work
piecemeal.  I actually started by just copying the C language definition
and then replacing individual fields as desired.

Now, here's some stuff I ran into that was less good.

* struct expression is very hard to deal with.  The prefixifying step is
  bewildering.  The need to manually manage slots in the expression
  makes all the code error-prone, especially because multiple bits of
  code all have to be modified in sync.  Adding a new operation involves
  making operator_length do the right thing, and also updating two
  different expression dumpers.  Expression evaluation has to handle two
  special "noside" cases, and one of them, AFAICT, is just fallout from
  the weird way that struct expression is laid out.

  I think replacing all of this with a more ordinary AST-like expression
  object would be a big improvement.

* I found val_print pretty ugly as well.  Printing a value means
  implementing a function that take 8 arguments, basically a struct
  value split into pieces.  Exactly what all of these mean is obscure,
  at least if you haven't been involved in the depths of gdb for a
  while...  back in the day I had wanted to replace this with a printing
  API that simply passed around values, not exploded values; I think
  that remains a good idea.

* Relatedly, it was a bit of a puzzle to build an object and fill it in.
  I think the choices were to use pack_* and fill in the bits manually,
  or allocate on the inferior, use value_assign to fields, and then
  re-fetch the resulting value.  Also only strings and arrays can be
  automatically pushed to inferior memory, so for struct objects I ended
  up requesting the allocation by hand.

* There's no built-in way to create a struct type and lay it out.  This
  requires a bit of arch support in the form of knowledge of any special
  ABI rules.

* The character and string printing hooks in struct language_defn are
  poorly factored.  Printing a string is very complicated, enough so
  that I didn't want to duplicate all the code in rust-lang.c.  However,
  despite the fact that there are two different character-printing
  hooks, it's still not possible to print character escapes in strings
  using Rust syntax.

* Writing a parser currently involves a bunch of global state.  There's
  struct parser_state but it doesn't contain all the relevant state,
  like innermost_block (this one particularly offends me for some
  reason) or parse_completion.

* There are a few spots that are desirable to update when adding a
  language that aren't covered by any hooks in language_defn.  In this
  series this includes the patches to dwarf2read; the adding of the
  ".rs" extension (file extensions could be in an array in the
  definition); and the change to symbol_find_demangled_name.

* Although DWARF encodes namespaces, I think there's no way to get at
  this information after gdb has made its symbol tables.  For Rust I
  ended up reusing some C++ functions to manipulate symbol names.

Tom


  reply	other threads:[~2016-04-27 16:22 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-27  2:50 Tom Tromey
2016-04-27  2:50 ` [PATCH 5/8] Add array start and end strings to generic_val_print_decorations Tom Tromey
2016-04-27 11:40   ` Pedro Alves
2016-04-27  2:50 ` [PATCH 7/8] Update gdb test suite for Rust Tom Tromey
2016-04-27 11:44   ` Pedro Alves
2016-04-27  2:50 ` [PATCH 4/8] Add self-test framework to gdb Tom Tromey
2016-04-27 11:40   ` Pedro Alves
2016-04-27 17:58     ` Tom Tromey
2016-04-27  2:50 ` [PATCH 1/8] Add DW_LANG_Rust and DW_LANG_Rust_old Tom Tromey
2016-04-27 11:44   ` Pedro Alves
2016-04-27  2:50 ` [PATCH 8/8] Add Rust documentation Tom Tromey
2016-04-27  7:10   ` Eli Zaretskii
2016-04-27  2:50 ` [PATCH 2/8] Fix latent yacc-related bug in gdb/Makefile.in init.c rule Tom Tromey
2016-04-27 11:38   ` Pedro Alves
2016-04-27  2:50 ` [PATCH 6/8] Add support for the Rust language Tom Tromey
2016-04-27 11:43   ` Pedro Alves
2016-04-27 11:49     ` Pedro Alves
2016-04-27 18:27     ` Tom Tromey
2016-04-27 18:36       ` Pedro Alves
2016-04-27 19:52       ` Tom Tromey
2016-04-27  2:50 ` [PATCH 3/8] Make gdb expression debugging handle OP_F90_RANGE Tom Tromey
2016-04-27 11:38   ` Pedro Alves
2016-04-27 11:47 ` [PATCH 0/8] Add Rust language support Pedro Alves
2016-04-27 16:22   ` Tom Tromey [this message]
2016-04-27 18:23     ` Pedro Alves
2016-04-27 21:56   ` Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87inz3c98x.fsf@tromey.com \
    --to=tom@tromey.com \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox