Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Tom Tromey <tromey@redhat.com>
To: Hilfinger@adacore.com
Cc: gdb-patches@sourceware.org
Subject: Re: [RFA] Rework symbol searches to move Ada-specific stuff to ada-lang.c.
Date: Wed, 06 Oct 2010 22:52:00 -0000	[thread overview]
Message-ID: <m3sk0j0wat.fsf@fleche.redhat.com> (raw)
In-Reply-To: <201010050842.o958gDNO002818@syracuse.mckusick.com> (Paul	Hilfinger's message of "Tue, 5 Oct 2010 01:42:13 -0700")

>>>>> "Paul" == Paul Hilfinger <hilfingr@syracuse.mckusick.com> writes:

Paul> Having added this general routine to psymtab.c, we use it in ada-lang.c
Paul> to rework add_non_local_symbols (now renamed add_nonlocal_symbols).

I don't think I should approve the ada-specific changes -- and I was
wrong to do so in my last review, I'll send a follow-up.

The psymtab / symfile / dwarf2read changes seem fine though.  I have a
few little nits.

Paul> +dw2_map_matching_symbols (const char* name, domain_enum namespace,

"const char *name"

Paul>  /* Lookup a partial symbol.  */
Paul> +static struct partial_symbol *match_partial_symbol (struct partial_symtab *,
Paul> +						    int,
Paul> +						    const char *, domain_enum,
Paul> +						    int (*) (const char *,
Paul> +							     const char *),
Paul> +						    int (*) (const char *,
Paul> +							     const char *));
Paul> +
Paul> +
Paul>  static struct partial_symbol *lookup_partial_symbol (struct partial_symtab *,

This insertion separates a comment from the declaration it comments.

I think it is fine to just remove that comment, though.  It doesn't
really add anything.

Paul> +/* Look in PST for a symbol in DOMAIN whose name matches NAME.  Search
Paul> +   the global block of PST if GLOBAL, and otherwise the static block.
Paul> +   MATCH is the comparison operation that returns true iff MATCH (s,
Paul> +   NAME), where s is a SYMBOL_SEARCH_NAME.  If ORDERED_COMPARE is
Paul> +   non-null, the symbols in the block are assumed to be ordered
Paul> +   according to it (allowing binary search).  It must be compatible
Paul> +   with MATCH.  Returns the symbol, if found, and otherwise NULL.  */
Paul> +static struct partial_symbol *
Paul> +match_partial_symbol (struct partial_symtab *pst, int global,

We've recently started trying to enforce the rule about having a blank
line between a function comment and the start of the function
definition.

Paul> +	    return (*top);

A few places have parens here, but they aren't needed.
Just write "return *top;".

Paul> +  /* Find global or static symbols in all tables that are in NAMESPACE 
Paul> +     and for which MATCH (symbol name, NAME) == 0, passing each to 
Paul> +     CALLBACK, reading in partial symbol symbol tables as needed.  Look
Paul> +     through global symbols if GLOBAL and otherwise static symbols.  
Paul> +     Passes NAME, NAMESPACE, and DATA to CALLBACK with each symbol
Paul> +     found.  After each block is processed, passes NULL to CALLBACK.
Paul> +     MATCH must be weaker than strcmp_iw in the sense that
Paul> +     strcmp_iw(x,y) == 0 --> MATCH(x,y) == 0.  ORDERED_COMPARE, if
Paul> +     non-null, must be an ordering relation compatible with strcmp_iw
Paul> +     in the sense that  
Paul> +            strcmp(x,y) == 0 --> ORDERED_COMPARE(x,y) == 0 
Paul> +     and 
Paul> +            strcmp(x,y) <= 0 --> ORDERED_COMPARE(x,y) <= 0
Paul> +     (allowing strcmp(x,y) < 0 while ORDERED_COMPARE(x, y) == 0). 
Paul> +     CALLBACK returns 0 to indicate that the scan should continue, or
Paul> +     non-zero to indicate that the scan should be terminated.  */

Thanks for being so thorough in this comment.

Tom


  reply	other threads:[~2010-10-06 22:52 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-05  8:42 Paul Hilfinger
2010-10-06 22:52 ` Tom Tromey [this message]
2010-10-07  4:14   ` Paul Hilfinger
2010-10-07 17:53     ` Tom Tromey
2010-10-07  7:19   ` [commit] " Paul Hilfinger
2010-10-06 23:15 ` [RFA] " Joel Brobecker
2010-10-06 23:49   ` Doug Evans
2010-10-07  4:18   ` Paul Hilfinger

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=m3sk0j0wat.fsf@fleche.redhat.com \
    --to=tromey@redhat.com \
    --cc=Hilfinger@adacore.com \
    --cc=gdb-patches@sourceware.org \
    /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