From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19046 invoked by alias); 18 Jan 2008 22:43:56 -0000 Received: (qmail 19025 invoked by uid 22791); 18 Jan 2008 22:43:53 -0000 X-Spam-Check-By: sourceware.org Received: from wa-out-1112.google.com (HELO wa-out-1112.google.com) (209.85.146.176) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 18 Jan 2008 22:43:32 +0000 Received: by wa-out-1112.google.com with SMTP id l35so1995763waf.12 for ; Fri, 18 Jan 2008 14:43:28 -0800 (PST) Received: by 10.114.154.1 with SMTP id b1mr1732537wae.34.1200696208640; Fri, 18 Jan 2008 14:43:28 -0800 (PST) Received: by 10.114.102.11 with HTTP; Fri, 18 Jan 2008 14:43:28 -0800 (PST) Message-ID: Date: Fri, 18 Jan 2008 22:43: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: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline 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/msg00483.txt.bz2 On 18/01/2008, Jim Blandy wrote: > > "Rob Quill" writes: > >> > + 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. > > I swear I'm not harrassing you. :) Once we can get these minor things > taken care of, the patch looks ready to me. Sorry about that. I was just frustrated because it feels like every time I submit this wrong it makes me look bad :) > The error should be indented two columns within the 'if'. Here's what > I see in most every other use of 'error' in the grammar section of the > file. > > if (new_type == NULL) > error ("No type \"%s\" within class or namespace \"%s\".", > ncopy, TYPE_NAME (type)); > > >> 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. > > Right you are. > > >> 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. > > This code block looks fine to me in your latest patch. > > The only other point left is the ChangeLog entry. The one you posted > was: > > 2008-01-17 Rob Quill > * c-exp.y : Add $in_scope as a type of expression. > > This needs a little rearrangement, and a bit more detail. A model to > follow might be: > > 2008-01-07 Vladimir Prus > > Ignore change in name of dynamic linker during > execution on Solaris. This also unbreaks pending breakpoints. > > * solist.h (struct target_so_ops): New field same. > * solib-svr4.c (svr4_same): New. > (_initialize_svr4_solib): Register svr4_same. > * solib.c (update_solib_list): Use ops->same, if available. > > If the change merits it, it's nice to have a (brief) summary sentence > at the top (substantial explanation belongs in comments in the code). > Your "Add $in_scope ..." might go there. Then, each change is > attributed to a particular function, top-level declaration, or > whatever grouping makes sense (in documentation, we put the section > name in the parens). > > So your ChangeLog entry would need to mention IN_SCOPE as a new token > you're defining, 'exp' (I guess) as the non-terminal symbol to which > you're adding a grammar rule, and 'yylex' as the function to which > you're adding a bit of code. Ok, here we go. Rob 2008-01-18 Rob Quill Add $in_scope as a type of expression. * c-exp.y (IN_SCOPE): New token. (exp): IN_SCOPE (name): evaluates to 1 if name is in scope, 0 otherwise. (yylex): Match "$in_scope" as IN_SCOPE token Index: gdb/c-exp.y =================================================================== RCS file: /cvs/src/src/gdb/c-exp.y,v retrieving revision 1.42 diff -u -p -r1.42 c-exp.y --- gdb/c-exp.y 9 Jan 2008 19:27:15 -0000 1.42 +++ gdb/c-exp.y 18 Jan 2008 22:41:05 -0000 @@ -208,6 +208,8 @@ static int parse_number (char *, int, in %token TRUEKEYWORD %token FALSEKEYWORD +/* $in_scope opperator */ +%token IN_SCOPE %left ',' %left ABOVE_COMMA @@ -251,6 +253,30 @@ exp1 : exp ; /* Expressions, not including the comma operator. */ +exp : IN_SCOPE '(' name_not_typename ')' + { + struct type *int_type; + struct minimal_symbol *min_symbol; + + /* If there are no symbols then just stop right away */ + if (!have_full_symbols () && !have_partial_symbols ()) + error ("No symbol table is loaded. Use the \"file\" command."); + + /* 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); } + ; + exp : '*' exp %prec UNARY { write_exp_elt_opcode (UNOP_IND); } ; @@ -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;