From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32611 invoked by alias); 25 Sep 2003 15:49:17 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 32592 invoked from network); 25 Sep 2003 15:49:11 -0000 Received: from unknown (HELO nevyn.them.org) (66.93.172.17) by sources.redhat.com with SMTP; 25 Sep 2003 15:49:11 -0000 Received: from drow by nevyn.them.org with local (Exim 4.22 #1 (Debian)) id 1A2YMX-00039S-T4; Thu, 25 Sep 2003 11:49:01 -0400 Date: Thu, 25 Sep 2003 15:49:00 -0000 From: Daniel Jacobowitz To: David Carlton Cc: gdb-patches@sources.redhat.com Subject: Re: [rfa] teach parser about C++ nested types Message-ID: <20030925154901.GA32104@nevyn.them.org> Mail-Followup-To: David Carlton , gdb-patches@sources.redhat.com References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.1i X-SW-Source: 2003-09/txt/msg00561.txt.bz2 On Wed, Sep 24, 2003 at 05:27:31PM -0700, David Carlton wrote: > A recap of the state of C++: our main goal is to embrace nested types. > So if you have a type (a class, a namespace, a typedef, whatever) D > defined within a class or namespace C, then the symbol associated to > that type should be called C::D, and our parsers, evaluators, > etc. should be happy about that. > > The current state of affairs is: right now we only generate symbols > with the right names if both C and D are namespaces, and the parsers, > etc. are fairly ignorant of the situation. So we have two issues to > deal with: > > 1) Generate all the symbols correctly. > 2) Tell the parsers, etc. to use them. > > This patch implements some of issue 2. This seems to be the correct > order: if we finish implementing issue 1 before doing any of issue 2, > probably the parser will get really confused, but fortunately the fact > that we've done just a bit of issue 1 (namely nested namespaces) means > that, if we implement issue 2 now, we can test it. > > Specifically, what this patch does is: > > * Move parsing of names containing '::' completely out of the lexer, > so it's all in the parser. As I comment in the code, this is a > lesser-of-two-evils choice; I've tried working on the lexer instead, > and it just didn't work as well. > > * Tell eval.c that, when it encounters an OP_SCOPE, to call a new > function value_aggregate_elt instead of > value_struct_elt_for_reference. > > * Teach value_aggregate_elt about namespaces. > > Some of this code will need to be slightly tweaked when we start > generating appropriately-named symbols for other nested types; this > gets the infrastructure in place, however. > > More details available on request. When reading through the code, be > alert for possible coding style mistakes I might have made: it's been > a few months since I've been immersed in GDB's house style, so I might > well have spaces in the wrong place or something. (Fortunately, most > of this code is cut and pasted from my branch, and was originally > written when I was immersed in GDB's house style.) > > Tested on i686-pc-linux-gnu, GCC 3.2, with DWARF 2, with a version > patched to generate DW_TAG_namespace, and with stabs. In no cases > were there any regressions; in both DWARF 2 cases, all the new tests > pass, while in the stabs case, all the new tests fail (which is > expected). > > Ok to commit? I think I only need approval from Daniel for this one; > it doesn't touch symtab stuff at all. For content, it's mostly fine. My only concern is about the reduce-reduce conflicts. How can this work without breaking one of the existing path or the new path? >From the bison manual: Bison resolves a reduce/reduce conflict by choosing to use the rule that appears first in the grammar, but it is very risky to rely on this. It took me quite a while to work out why this works at all, so a comment, please. The reason it works (roughly; please don't point out flaws in my grasp of parsing, which I know is shaky :) is that we don't know whether we want a qualified name or a qualified type, so we choose a qualified name per the rule above. Then to the left of the final colon we know we need a type so we choose qualified_type. But for the first item, qualified_type or qualified_name should work. I'm uncomfortable about how AAA::inA and the AAA::inA portion of AAA::inA::fum get parsed differently. I wonder if sharing the parser with GCC some day is workable... without being a maintenance problem for GCC. It seems dubious and the reaction when I suggested it involved a lot of sniggering. Maybe at first forking said parser. [One reason I'd like to do this is to parse statements. Which puts us on extremely shaky ground. But macros can expand to gcc statement-expressions, and it would be really, really cool to be able to handle that.] For style, two small problems: > @@ -1687,7 +1744,16 @@ yylex () > > if (sym && SYMBOL_CLASS (sym) == LOC_TYPEDEF) > { > -#if 1 > + /* FIXME: carlton/2003-09-24: I've turned off the code > + below, because it seems to me to work better to deal with > + nested types in the parser instead of the lexer. See the > + comment before qualified_type for more info. If you're > + interested in figuring out the code below, be aware that > + it dates from a time when we handled nested types very > + differently than we do now, and I don't believe the > + comments within it had been entirely accurate for a > + while. */ > +#if 0 > /* Despite the following flaw, we need to keep this code enabled. > Because we can get called from check_stub_method, if we don't > handle nested types then it screws many operations in any I'm a big deleter-of-code. What about leaving a comment here and getting rid of the old code? It's not like we'll want it back. Something mentioning how qualified names used to be handled here. > + default: > + error ("Internal error: non-aggregate type in value_aggregate_elt"); Please use internal_error. With the style corrections and another big fat comment in the parser about why the reduce/reduce conflict is almost OK, this is approved. -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer