Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: Paul Hilfinger <Hilfinger@adacore.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [RFA/Ada] Allow 'thread' to be used as a variable name in expressions.
Date: Sun, 10 Mar 2013 15:33:00 -0000	[thread overview]
Message-ID: <20130310153348.GC3264@adacore.com> (raw)
In-Reply-To: <20130310075951.B94553FF09@kwai.gnat.com>

Hi Paul,

> gdb/ChangeLog:
> 
>     * ada-lex.l (rules): Only recognize 'thread' as a
>       delimiter when followed by numerals, as for c-exp.y.
>       Use new rewind_to_char function to rewind the input for
>       expression-delimiting tokens.
>       (rewind_to_char): New function.

Just a few nits... The text in the ChangeLog entry should be
left-aligned on the tab, so:

      * ada-lex.l (rules): Only recognize 'thread' as a
      delimiter when followed by numerals, as for c-exp.y.
      Use new rewind_to_char function to rewind the input for
      expression-delimiting tokens.
      (rewind_to_char): New function.

There are a few formatting issues that I will highlight below,
but also, could you add a couple of regression tests?

> +static void rewind_to_char(int);

Space before '('.

> +/* Back up lexptr by yyleng and then to the rightmost occurrence of character
> +   CH, case-folded (there must be one).  WARNING: since lexptr points to the
> +   next input character that Flex has not yet transferred to its
> +   internal buffer, the use of this function depends on the assumption
> +   that Flex calls YY_INPUT only when it is logically necessary to do so
> +   (thus, there is no reading ahead farther than needed to identify
> +   the next token.)  */

Would you mind reformatting this comment to 70 characters only?
Exceeding that limit up to 80 is acceptable if it helps readability.

> +static void
> +rewind_to_char(int ch)

Missing space before '('.

> +{
> +  lexptr -= yyleng;
> +  while (toupper(*lexptr) != toupper(ch))

Likewise...

Thank you,
-- 
Joel


  reply	other threads:[~2013-03-10 15:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-10  8:00 Paul Hilfinger
2013-03-10 15:33 ` Joel Brobecker [this message]
2013-03-11  8:37   ` Paul Hilfinger
2013-03-11 17:57     ` Joel Brobecker
2013-03-12  9:20   ` [commit] " 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=20130310153348.GC3264@adacore.com \
    --to=brobecker@adacore.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