Re: [rfa] teach parser about C++ nested types

[Daniel Jacobowitz <drow@mvista.com>]

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