From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6481 invoked by alias); 17 Jan 2008 19:32:52 -0000 Received: (qmail 6339 invoked by uid 22791); 17 Jan 2008 19:32:51 -0000 X-Spam-Check-By: sourceware.org Received: from nz-out-0506.google.com (HELO nz-out-0506.google.com) (64.233.162.225) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 17 Jan 2008 19:32:29 +0000 Received: by nz-out-0506.google.com with SMTP id x7so576238nzc.3 for ; Thu, 17 Jan 2008 11:32:27 -0800 (PST) Received: by 10.115.106.7 with SMTP id i7mr90152wam.18.1200598347091; Thu, 17 Jan 2008 11:32:27 -0800 (PST) Received: by 10.114.92.3 with HTTP; Thu, 17 Jan 2008 11:32:27 -0800 (PST) Message-ID: Date: Thu, 17 Jan 2008 19:32:00 -0000 From: "Rob Quill" To: "Jim Blandy" Subject: Re: New scope checking patch Cc: msnyder@specifix.com, gdb-patches@sourceware.org In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_Part_1310_24055401.1200598347082" References: 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/msg00453.txt.bz2 ------=_Part_1310_24055401.1200598347082 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 3573 On 11/01/2008, Jim Blandy wrote: > > "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 > Sorry about that, the attached patch is done with cvs diff -up > >> 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. > Thanks for the tip :) I think the only example I had seen of parsing actual values was where it used the parse number function, so I assumed that was the right thing to use. > > 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. My mistake, I didn't realise before that keywords were declared here. Rob ------=_Part_1310_24055401.1200598347082 Content-Type: text/x-diff; name=in_scope.patch Content-Transfer-Encoding: base64 X-Attachment-Id: f_fbjp7e1t Content-Disposition: attachment; filename=in_scope.patch Content-length: 2095 SW5kZXg6IGdkYi9jLWV4cC55Cj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KUkNT IGZpbGU6IC9jdnMvc3JjL3NyYy9nZGIvYy1leHAueSx2CnJldHJpZXZpbmcg cmV2aXNpb24gMS40MgpkaWZmIC11IC1yMS40MiBjLWV4cC55Ci0tLSBnZGIv Yy1leHAueQk5IEphbiAyMDA4IDE5OjI3OjE1IC0wMDAwCTEuNDIKKysrIGdk Yi9jLWV4cC55CTEzIEphbiAyMDA4IDIwOjIyOjI2IC0wMDAwCkBAIC0yMDgs NiArMjA4LDggQEAKICV0b2tlbiBUUlVFS0VZV09SRAogJXRva2VuIEZBTFNF S0VZV09SRAogCisvKiAkaW5fc2NvcGUgb3BwZXJhdG9yICovCislbGVmdCBJ Tl9TQ09QRQogCiAlbGVmdCAnLCcKICVsZWZ0IEFCT1ZFX0NPTU1BCkBAIC0y NTEsNiArMjUzLDMyIEBACiAJOwogCiAvKiBFeHByZXNzaW9ucywgbm90IGlu Y2x1ZGluZyB0aGUgY29tbWEgb3BlcmF0b3IuICAqLworZXhwCTogSU5fU0NP UEUgJygnIG5hbWVfbm90X3R5cGVuYW1lICcpJworCXsKKwkgIHN0cnVjdCB0 eXBlICppbnRfdHlwZTsKKworCSAgLyogSWYgdGhlcmUgYXJlIG5vIHN5bWJv bHMgdGhlbiBqdXN0IHN0b3AgcmlnaHQgYXdheSAqLworCSAgaWYgKCFoYXZl X2Z1bGxfc3ltYm9scyAoKSAmJiAhaGF2ZV9wYXJ0aWFsX3N5bWJvbHMgKCkp CisJCWVycm9yICgiTm8gc3ltYm9sIHRhYmxlIGlzIGxvYWRlZC4gIFVzZSB0 aGUgXCJmaWxlXCIgY29tbWFuZC4iKTsKKworCSAgLyogT3RoZXJ3aXNlLCBw cmVwYXJlIHRvIHdyaXRlIG91dCB0aGUgdmFsdWUgKi8KKyAgICAgIGludF90 eXBlID0gYnVpbHRpbl90eXBlIChjdXJyZW50X2dkYmFyY2gpLT5idWlsdGlu X2ludDsKKyAgICAgIHdyaXRlX2V4cF9lbHRfb3Bjb2RlIChPUF9MT05HKTsK KyAgICAgIHdyaXRlX2V4cF9lbHRfdHlwZSAoaW50X3R5cGUpOworCisJICBp ZiAoJDMuc3ltIHx8IGxvb2t1cF9taW5pbWFsX3N5bWJvbChjb3B5X25hbWUo JDMuc3Rva2VuKSwgTlVMTCwgTlVMTCkpCisJICB7CisgICAgICAgIHdyaXRl X2V4cF9lbHRfbG9uZ2NzdCAoKExPTkdFU1QpIDEpOworCSAgfQorCSAgZWxz ZQorCSAgeworCSAgICB3cml0ZV9leHBfZWx0X2xvbmdjc3QgKChMT05HRVNU KSAwKTsKKwkgIH0KKworCSAgd3JpdGVfZXhwX2VsdF9vcGNvZGUgKE9QX0xP TkcpOworCX0KKwk7CisKIGV4cAk6CScqJyBleHAgICAgJXByZWMgVU5BUlkK IAkJCXsgd3JpdGVfZXhwX2VsdF9vcGNvZGUgKFVOT1BfSU5EKTsgfQogCTsK QEAgLTE2NzgsNiArMTcwNiw5IEBACiAgIC8qIENhdGNoIHNwZWNpZmljIGtl eXdvcmRzLiAgU2hvdWxkIGJlIGRvbmUgd2l0aCBhIGRhdGEgc3RydWN0dXJl LiAgKi8KICAgc3dpdGNoIChuYW1lbGVuKQogICAgIHsKKwljYXNlIDk6CisJ ICBpZiAoc3RybmNtcCAodG9rc3RhcnQsICIkaW5fc2NvcGUiLCA5KSA9PSAw KQorCXJldHVybiBJTl9TQ09QRTsKICAgICBjYXNlIDg6CiAgICAgICBpZiAo c3RybmNtcCAodG9rc3RhcnQsICJ1bnNpZ25lZCIsIDgpID09IDApCiAJcmV0 dXJuIFVOU0lHTkVEOwo= ------=_Part_1310_24055401.1200598347082--