From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30818 invoked by alias); 18 Jan 2008 03:35:58 -0000 Received: (qmail 30810 invoked by uid 22791); 18 Jan 2008 03:35:57 -0000 X-Spam-Check-By: sourceware.org Received: from wa-out-1112.google.com (HELO wa-out-1112.google.com) (209.85.146.177) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 18 Jan 2008 03:35:35 +0000 Received: by wa-out-1112.google.com with SMTP id l35so1462381waf.12 for ; Thu, 17 Jan 2008 19:35:34 -0800 (PST) Received: by 10.114.168.1 with SMTP id q1mr790385wae.74.1200627333837; Thu, 17 Jan 2008 19:35:33 -0800 (PST) Received: by 10.114.92.3 with HTTP; Thu, 17 Jan 2008 19:35:33 -0800 (PST) Message-ID: Date: Fri, 18 Jan 2008 03:35: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_2770_28901290.1200627333833" 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/msg00467.txt.bz2 ------=_Part_2770_28901290.1200627333833 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 3914 On 17/01/2008, Jim Blandy wrote: > > "Rob Quill" writes: > > On 17/01/2008, Jim Blandy wrote: > >> Also, please be sure that the indentation follows the GNU coding > >> conventions. Substatements should be indented by two spaces. > >> (c-exp.y is not a great place to look for examples, since it's a mess, > >> but look at, say, frame.c.) > >> > >> From looking at your patch as it arrived through my mailer, it seemed > >> that the code block for the new $in_scope grammar rule was not > >> indented in the same way as the other blocks. These should all be > >> consistent. > > > > Hey, > > > > Sorry about getting the formatting consistently wrong. I've > > reformatted it and it looks to me like it matches the other cases, > > although it is hard to tell where to use tabs and where to use spaces, > > and I'm never sure it's right as what if I have a different tab size > > to you etc. I tried putting it through indent, but that just made a > > mess, although as a rule is it OK to use that if it is a C file? > > I'm told Open Source projects avoid tabs, but it seems that GDB > permits them, with tab stops every 8 columns. The GNU coding > standards don't say much about tab use or width, beyond saying that > error messages that include line and column numbers should assume tab > stops every 8 columns when computing column numbers. > > We don't generally use indent; if you look through the mailing list > archives, you can see the arguments. I can only remember the > arguments that made sense to me, so I'm not sure I can accurately > explain the reasoning. :) > > If you use GNU Emacs C mode with the default settings, then TAB, C-j, > and C-M-q will do the right thing. > > > + if (!have_full_symbols () && !have_partial_symbols ()) > > + error ("No symbol table is loaded. Use the \"file\" command."); > > Too much indentation? Well, you would think so, but I've changed the tab spacing in vim to 8 and the way I have formatted it matches how everything else is formatted. > > + > > + /* Otherwise, prepare to write out the value */ > > + int_type = builtin_type (current_gdbarch)->builtin_int; > > + write_exp_elt_opcode (OP_LONG); > > + write_exp_elt_type (int_type); > > + > > + min_symbol = > > + lookup_minimal_symbol (copy_name($3.stoken), NULL, NULL); > > + if ($3.sym || min_symbol) > > + write_exp_elt_longcst ((LONGEST) 1); > > + else > > + write_exp_elt_longcst ((LONGEST) 0); > > > > > + > > + write_exp_elt_opcode (OP_LONG); } > > Closing brace should get its own line, since the opening brace did. This is what I thought too, but if you look at the rest of the file (for example the case for "exp : exp ARROW name") the open brace always starts on a new line and always ends on the same line. > > > @@ -1678,6 +1704,9 @@ yylex () > > /* Catch specific keywords. Should be done with a data structure. */ > > switch (namelen) > > { > > + case 9: > > + if (strncmp (tokstart, "$in_scope", 9) == 0) > > + return IN_SCOPE; > > case 8: > > if (strncmp (tokstart, "unsigned", 8) == 0) > > return UNSIGNED; > > When I view this (again, with 8-column tab stops), the 'case' is not > lined up with the other cases, and the 'return' is not indented two > spaces within the 'if'. Sorry, missed that. I've attached the fixed copy. I believe that the amount of indentation matches the layout of the current file, and that even though it looks like a lot in the context of the patch, all the other grammar rules in the file use the same large amount of indentation. Rob ------=_Part_2770_28901290.1200627333833 Content-Type: text/x-diff; name=in_scope.patch Content-Transfer-Encoding: base64 X-Attachment-Id: f_fbk6icrv Content-Disposition: attachment; filename=in_scope.patch Content-length: 2290 SW5kZXg6IGdkYi9jLWV4cC55Cj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KUkNT IGZpbGU6IC9jdnMvc3JjL3NyYy9nZGIvYy1leHAueSx2CnJldHJpZXZpbmcg cmV2aXNpb24gMS40MgpkaWZmIC11IC1wIC1yMS40MiBjLWV4cC55Ci0tLSBn ZGIvYy1leHAueQk5IEphbiAyMDA4IDE5OjI3OjE1IC0wMDAwCTEuNDIKKysr IGdkYi9jLWV4cC55CTE4IEphbiAyMDA4IDAzOjMyOjQxIC0wMDAwCkBAIC0y MDgsNiArMjA4LDggQEAgc3RhdGljIGludCBwYXJzZV9udW1iZXIgKGNoYXIg KiwgaW50LCBpbgogJXRva2VuIFRSVUVLRVlXT1JECiAldG9rZW4gRkFMU0VL RVlXT1JECiAKKy8qICRpbl9zY29wZSBvcHBlcmF0b3IgKi8KKyV0b2tlbiBJ Tl9TQ09QRQogCiAlbGVmdCAnLCcKICVsZWZ0IEFCT1ZFX0NPTU1BCkBAIC0y NTEsNiArMjUzLDMwIEBAIGV4cDEJOglleHAKIAk7CiAKIC8qIEV4cHJlc3Np b25zLCBub3QgaW5jbHVkaW5nIHRoZSBjb21tYSBvcGVyYXRvci4gICovCitl eHAJOglJTl9TQ09QRSAnKCcgbmFtZV9ub3RfdHlwZW5hbWUgJyknCisJCQl7 CisJCQkgIHN0cnVjdCB0eXBlICppbnRfdHlwZTsKKwkJCSAgc3RydWN0IG1p bmltYWxfc3ltYm9sICptaW5fc3ltYm9sOworCisJCQkgIC8qIElmIHRoZXJl IGFyZSBubyBzeW1ib2xzIHRoZW4ganVzdCBzdG9wIHJpZ2h0IGF3YXkgKi8K KwkJCSAgaWYgKCFoYXZlX2Z1bGxfc3ltYm9scyAoKSAmJiAhaGF2ZV9wYXJ0 aWFsX3N5bWJvbHMgKCkpCisJCQkJZXJyb3IgKCJObyBzeW1ib2wgdGFibGUg aXMgbG9hZGVkLiAgVXNlIHRoZSBcImZpbGVcIiBjb21tYW5kLiIpOworCisJ CQkgIC8qIE90aGVyd2lzZSwgcHJlcGFyZSB0byB3cml0ZSBvdXQgdGhlIHZh bHVlICovCisJCQkgIGludF90eXBlID0gYnVpbHRpbl90eXBlIChjdXJyZW50 X2dkYmFyY2gpLT5idWlsdGluX2ludDsKKwkJCSAgd3JpdGVfZXhwX2VsdF9v cGNvZGUgKE9QX0xPTkcpOworCQkJICB3cml0ZV9leHBfZWx0X3R5cGUgKGlu dF90eXBlKTsKKworCQkJICBtaW5fc3ltYm9sID0gCisJCQkgICAgbG9va3Vw X21pbmltYWxfc3ltYm9sIChjb3B5X25hbWUoJDMuc3Rva2VuKSwgTlVMTCwg TlVMTCk7CisJCQkgIGlmICgkMy5zeW0gfHwgbWluX3N5bWJvbCkKKwkJCSAg ICB3cml0ZV9leHBfZWx0X2xvbmdjc3QgKChMT05HRVNUKSAxKTsKKwkJCSAg ZWxzZQorCQkJICAgIHdyaXRlX2V4cF9lbHRfbG9uZ2NzdCAoKExPTkdFU1Qp IDApOworCisJCQkgIHdyaXRlX2V4cF9lbHRfb3Bjb2RlIChPUF9MT05HKTsg fQorCTsKKwogZXhwCToJJyonIGV4cCAgICAlcHJlYyBVTkFSWQogCQkJeyB3 cml0ZV9leHBfZWx0X29wY29kZSAoVU5PUF9JTkQpOyB9CiAJOwpAQCAtMTY3 OCw2ICsxNzA0LDkgQEAgeXlsZXggKCkKICAgLyogQ2F0Y2ggc3BlY2lmaWMg a2V5d29yZHMuICBTaG91bGQgYmUgZG9uZSB3aXRoIGEgZGF0YSBzdHJ1Y3R1 cmUuICAqLwogICBzd2l0Y2ggKG5hbWVsZW4pCiAgICAgeworICAgIGNhc2Ug OToKKyAgICAgIGlmIChzdHJuY21wICh0b2tzdGFydCwgIiRpbl9zY29wZSIs IDkpID09IDApCisJcmV0dXJuIElOX1NDT1BFOwogICAgIGNhc2UgODoKICAg ICAgIGlmIChzdHJuY21wICh0b2tzdGFydCwgInVuc2lnbmVkIiwgOCkgPT0g MCkKIAlyZXR1cm4gVU5TSUdORUQ7Cg== ------=_Part_2770_28901290.1200627333833--