Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@gnat.com>
To: Jim Blandy <jimb@redhat.com>
Cc: Andrew Cagney <ac131313@redhat.com>,
	Paul Koning <pkoning@equallogic.com>,
	gdb-patches@sources.redhat.com
Subject: Re: [RFA] parse and eval breakpoint conditions with correct language
Date: Mon, 15 Sep 2003 19:00:00 -0000	[thread overview]
Message-ID: <20030915190054.GG15984@gnat.com> (raw)
In-Reply-To: <vt21xuin41l.fsf@zenia.home>

> What gives me pause, though, is that the breakpoint_re_set_one isn't
> calling parse_exp_1 directly --- it's calling decode_line_1.  Are you
> going to propagate the language all the way through there?

Yes, it's the only approach I see.

> How do you see approaching this?

I think the safest way to approach this is by working from the bottom to
the top, in tiny steps that should be backward compatible in terms of
functionality.

Let's define the objectives:

  1. We want parse_exp_1 to take a language argument, so that it can
     use it to do the expression parsing.

  2. We want to take advantage of it when parsing a breakpoint condition
     For that, we need to be able to determine the language associated
     to a given block (or a given PC, whatever is more convenient)

  3. We want to take advantage of 1 to make sure we use the same language
     everytime we reparse the condition.

  4. We want decode_line_1 to take a language argument too, so that when
     we reset the breakpoints, we can use the same language as the
     language that was used when the breakpoint was first created.

  5. We want to fix the watchpoint problem that we also noticed.

I think these 5 steps can be done separately. Here is the plan:

  1. Add a language parameter to parse_exp_1. If NULL, then use
     current_language. Fix all the calls the parse_exp_1 to use
     NULL. As is, this change should be a nop.

  2. Pass the appropriate language to parse_exp_1 in the instances
     where we parse a condition. That includes modifying
     condition_command(), break_command_1(), do_captured_breakpoints().
     These changes are self-contained, as these procedures can determine
     on their own the appropriate language. This should fix the original
     problem I reported.

  3. Fix the call to parse_exp_1 in breakpoint_re_set_one to use the
     same language to reparse the condition. This is also
     self-contained.

  4. a. Add a language argument to decode_line_1() using the same
        nop approach as for 1.
     b. Fix the call to decode_line_1 in breakpoint_re_set_one so
        that the appropriate language is used to parse the given
        expression. That will allow us to remove the call to
        set_language, and will make sure that the second problem
        that I reported will not re-appear.
  
  5. Fix the problem of the wrong language used to reinsert watchpoints.
     For that we need to do:
     
     a. Add a language paramter to parse_expression(). Same as 1.
     b. Fix the call to parse_expression in breakpoint_re_set_one()
        to use the same language as before when reparsing the watchpoint
        expression.
  
The downside is that we will not end up reviewing the entire calling
tree to parse_exp_1, so we may leave some problems behind. But whatever
problems they are, we will be no worse than before. And the upside is
that a lot of the changes should be mechanical, and most of the patches
should be reasonably small and easy to review.
     
Shall I do that?
-- 
Joel


  reply	other threads:[~2003-09-15 19:00 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-09-10  1:54 Joel Brobecker
2003-09-10 17:31 ` Jim Blandy
2003-09-11 18:09   ` Joel Brobecker
2003-09-11 18:42     ` Paul Koning
2003-09-11 19:30       ` Joel Brobecker
2003-09-12  1:33         ` Andrew Cagney
2003-09-12  5:46           ` Joel Brobecker
2003-09-15  3:09             ` Jim Blandy
2003-09-15 19:00               ` Joel Brobecker [this message]
2003-09-11 19:20     ` Andrew Cagney
2003-09-11 19:32       ` Joel Brobecker
2003-09-11 19:54       ` Daniel Jacobowitz
2003-09-11 22:20     ` Jim Blandy

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=20030915190054.GG15984@gnat.com \
    --to=brobecker@gnat.com \
    --cc=ac131313@redhat.com \
    --cc=gdb-patches@sources.redhat.com \
    --cc=jimb@redhat.com \
    --cc=pkoning@equallogic.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