From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19437 invoked by alias); 9 Oct 2008 22:20:34 -0000 Received: (qmail 19411 invoked by uid 22791); 9 Oct 2008 22:20:33 -0000 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 09 Oct 2008 22:19:33 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 4CF821EE116; Thu, 9 Oct 2008 18:19:31 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id emIO0f16e+oY; Thu, 9 Oct 2008 18:19:31 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 028D61EE112; Thu, 9 Oct 2008 18:19:30 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 28CFBE7ACD; Thu, 9 Oct 2008 18:19:30 -0400 (EDT) Date: Thu, 09 Oct 2008 22:20:00 -0000 From: Joel Brobecker To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [RFC/RFA] add struct parse_context to all command functions Message-ID: <20081009221930.GG3810@adacore.com> References: <20081009140424.GD5372@adacore.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.2i Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-10/txt/msg00307.txt.bz2 > ... but I didn't see an explanation of the problem. Would you mind > repeating it? Sure, http://www.sourceware.org/ml/gdb-patches/2007-10/msg00181.html: > There is a project that I'm itching to start is to rework a bit our > expression evaluation interfaces to use an explicit language rather > than relying on the current_language global. The reason for this is > that it will make it much clearer which language is to be used and > will prevent some oopsies that can appear during situations like: > > breakpoint_re_set_one > -> set language to breakpoint language > -> re-evaluation breakpoint location > -> reset language to intial value > > I have seen some cases, especially during the inferior startup > phase, where we inadvertandly switch the language to an irrelevant > value because as a side-effect of calling "select_frame ()". As > a result, we end up evaluating the breakpoint location using > the wrong language! > > On mips-irix, we end up getting errors like this: > > % gdb foo > (gdb) b foo > Breakpoint 1 at 0x1000278c: file foo.adb, line 4. > (gdb) run > Starting program: /kern.a/brobecke/head/ex/foo > Error in re-setting breakpoint 1: > Function "foo" not defined. > > Program exited normally. > > I think it's going to be a lot cleaner to pass a specific language > to the parser/evaluator rather having it use the current language. > And it's going to help us fix that problem above. Right now, I'm > not sure I can find a solution as we have done a few times in the > past already. > struct cmd_list_element already has some support for multiple styles > of callback. It seems to me that you could limit your change to a > subset of all the commands by adding a new field to the 'function' > union. (That would mean more add_* functions, though.) Do you mean adding new "add_..." commands, and transitionning the old ones to the new ones gradually? That should work indeed - the trickiness is related to the fact that some of the command functions are used for more than one commands through "add_cmd" and "add_prefix_cmd". For instance in breakpoint.c: add_prefix_cmd ("enable", class_breakpoint, enable_command, ...); if (xdb_commands) add_com ("ab", class_breakpoint, enable_command, ...); If I provide replacement versions for all the add_... commands right from the start, it should allow us to transition gradually. I could propose the following: . First patch: Rename all the add_... functions into add_..._nopc (for "NO Parse Context"). That's a mega-patch, but should be automatable. . Second patch: Add the add_... functions back, with the new interface. . Followup patches: Transition each file one after the other. . Final patch: When the old functions are no longer used, we remove them. I'm Ok with this either the initial approach where everything is transitionned all at once. I don't really prefer the gradual transition because it doesn't avoid the mega patch issue, makes the transition last over a longer duration, and is in fact a little more work for me. But I don't mind using the gradual approach is people prefer that. -- Joel