public inbox for archer@sourceware.org
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: Keith Seitz <keiths@redhat.com>
Cc: archer@sourceware.org
Subject: Re: [Archer] Stop the Insanity! Linespec Rewrite
Date: Fri, 02 Mar 2012 19:07:00 -0000	[thread overview]
Message-ID: <20120302190658.GH2867@adacore.com> (raw)
In-Reply-To: <4F501EC1.3050104@redhat.com>

> Earlier this month, I dug this out of mothballs, and it is now time
> to submit the work-in-progress for comment. This is *not* a finished
> design and/or implementation! It is simply a place to stop along the
> road and ask for advice, for a fresh set of eyes. Consider this akin
> to an RFC.

I really like the fact that you're now splitting the parsing from the
evaluation (the transformation into SALs).

Looking at the parsing code, I had a feeling that it would not work
with Ada operators, and in particular with:

        (gdb) break "*"
        Argument required (expression to compute).

I then tried other operators that might be problematic, eg:

        (gdb) break "+"
        Function "+" not defined.
        Make breakpoint pending on future shared library load

But also simpler-to-handle operators such as "/" do not seem to work:

        (gdb) braek "/"
        Function "/" not defined.
        Make breakpoint pending on future shared library load

I reviewed the gdb.ada testsuite, and it appears that I failed to
add a testcase for those. I will put that on my list.

I can help with the implementation side of the above feature, but
do you think your parser could accomodate that? As discussed with
Tom, it is fine if we need to compromise a bit an accept "break *"
(the operator name without quotes) or "break '*'" (operator name
bracketed by single-quotes, which is illegal Ada, but who cares).

I have noticed a few other things:

  . Very minor: We now accept the "task" keyword in any casing (?).
    So now "break foo TASK 3" is accepted, whereas it wasn't in the past.

  . Your new linespec passes the entire gdb.ada testsuite. Congrats,
    this is no small feat, if you ask me!

  . Our testsuite spotted a couple of crashes. They might be related
    to the crash that Tom mentioned, although I kind of doubt it.
    Nevertheless, I'll investigate them after you've looked at Tom's
    report, just in case they end up having the same cause.

The code itself looks pretty good to me, although I mostly skimmed
through it. I agree with Tom that you can think of cleaning things up
and do an official submission. I'd like to resolve the issues mentioned
above before the patch actually goes in, but I have spare cycles next
week for that - let's enjoy those spares cycles while they last. They
have been pretty rare lately...

-- 
Joel

  parent reply	other threads:[~2012-03-02 19:07 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-02  1:14 Keith Seitz
2012-03-02 17:00 ` Tom Tromey
2012-03-02 22:41   ` Keith Seitz
2012-03-03  2:36     ` Tom Tromey
2012-03-02 19:07 ` Joel Brobecker [this message]
2012-03-02 22:49   ` [Archer] " Keith Seitz
2012-03-06 17:40     ` [Archer] " Joel Brobecker
2012-03-06 19:08       ` Keith Seitz
2012-03-06 19:36         ` Keith Seitz
2012-03-06 21:50           ` [Archer] " Joel Brobecker
2012-03-07  0:11             ` [Archer] " Joel Brobecker
2012-03-07  1:08               ` Keith Seitz
2012-03-07  1:26                 ` [Archer] " Joel Brobecker
2012-03-07 14:39                   ` Tom Tromey
2012-03-07 15:01                     ` Tom Tromey
2012-03-07 15:39                       ` [Archer] " Joel Brobecker
2012-03-07 16:06                         ` Tom Tromey
2012-03-07 21:19                           ` Joel Brobecker
2012-03-07 21:11                         ` [Archer] " Keith Seitz
2012-03-08  3:02                           ` Joel Brobecker

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=20120302190658.GH2867@adacore.com \
    --to=brobecker@adacore.com \
    --cc=archer@sourceware.org \
    --cc=keiths@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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).