From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28582 invoked by alias); 11 Jan 2008 22:51:19 -0000 Received: (qmail 28571 invoked by uid 22791); 11 Jan 2008 22:51:18 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 11 Jan 2008 22:51:01 +0000 Received: (qmail 5979 invoked from network); 11 Jan 2008 22:50:59 -0000 Received: from unknown (HELO localhost) (jimb@127.0.0.2) by mail.codesourcery.com with ESMTPA; 11 Jan 2008 22:50:59 -0000 To: "Rob Quill" Cc: msnyder@specifix.com, gdb-patches@sourceware.org Subject: Re: New scope checking patch References: From: Jim Blandy Date: Fri, 11 Jan 2008 22:51:00 -0000 In-Reply-To: (Rob Quill's message of "Fri, 11 Jan 2008 00:52:38 +0000") Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-IsSubscribed: yes 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-01/txt/msg00297.txt.bz2 "Rob Quill" writes: > Please find attached the revised and much simpler patch. One of the > drawbacks of the new patch is that you cannot check the scope of > structure members, which is quite inconvenient, but I can't really fix > that without resorting to the old style of patch. > > I think that for the moment this patch is sufficient and definitely > provides and improvement, but I think in future it would be good to be > able to check arbitrary expressions, but as you say in your previous > email, this requires some significant reworking of the patch. It also > depends on how much people will find it useful. Michael, how do you > feel about this? (As you seemed interested in the previous attempt) If > there is enough interest then I am more than happy to spend some time > reworking the old patch at a later date. Unfortunately, I can't try out this patch. Absurdly, the default output format for 'diff' can't be used reliably by patch, because it doesn't include any context lines (unchanged text surrounding the changed text), which 'patch' uses to ensure that the change doesn't conflict with other changes made to the code. Use 'cvs diff -u' or 'cvs diff -c' to produce unified or context diffs. Unified diffs seem to be the more widely preferred form these days. All patches posted to this list (or pretty much to any other open source list) should be in unified or context diff form. I have the following in my ~/.cvsrc file, so I don't have to remember to type '-u' all the time: diff -u >> parse_number ("1", 1, 0, &val); >> write_exp_elt_opcode (OP_LONG); >> write_exp_elt_type (val.typed_val_int.type); >> write_exp_elt_longcst ((LONGEST)val.typed_val_int.val); >> write_exp_elt_opcode (OP_LONG); You've written out this code three times; that's quite a chunk. I'd recommend first deciding whether the symbol is in scope (checking '$3.sym', and then calling lookup_minimal_symbol, as you do now), and computing a one or a zero from that. Then, did you know that you can get away with, simply: struct type *int_type = builtin_type (current_gdbarch)->builtin_int; write_exp_elt_opcode (OP_LONG); write_exp_elt_type (int_type); write_exp_elt_longcst (); write_exp_elt_opcode (OP_LONG); So there's no need to parse the number from a string. > 1313a1357,1361 >> static const struct token tokentab9[] = >> { >> {"$in_scope", IN_SCOPE, BINOP_END} >> }; >> > 1375a1424,1432 >> /* Code for recognising the $in_scope token. */ >> /* See if it is a special token of length 9. */ >> for (i = 0; i < sizeof tokentab9 / sizeof tokentab9[0]; i++) >> if (strncmp (tokstart, tokentab9[i].operator, 9) == 0) >> { >> lexptr += 9; >> yylval.opcode = tokentab9[i].opcode; >> return tokentab9[i].token; >> } I think I would rather see $in_scope recognized like the other keywords (see "Catch specific keywords.") than at this point as a symbolic token.