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

[David Carlton <carlton at kealia dot com>]

On Thu, 25 Sep 2003 11:49:01 -0400, Daniel Jacobowitz <drow@mvista.com> said:

> 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.

That's my understanding, too.

> I'm uncomfortable about how AAA::inA and the AAA::inA portion of
> AAA::inA::fum get parsed differently.

Yeah, really.  I should also mention in the comment another aspect of
the reason why this works: we already have this handy 'noside'
variable floating around in the evaluator, which basically tells us
whether we're only looking for type info or really need an actual
expression, so as long as we pay attention to that, it doesn't really
matter too much if, in 'ptype AAA::inA', the parser thinks that
AAA::inA is an expression or a type.  (I can't figure out how much of
this is good design and how much of this is a hack to get around bad
design, but it works pretty well either way.)

> 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.

Sure.  I'm just so used to having Elena in charge of approving my
patches that my reflex is to never delete code any more. :-)

>> +    default:
>> +      error ("Internal error: non-aggregate type in value_aggregate_elt");

> Please use internal_error.

Right, will do.

> With the style corrections and another big fat comment in the parser
> about why the reduce/reduce conflict is almost OK, this is approved.

Thanks!  Geez, now I'll feel guilty if I don't generate my next patch
quickly. :-)

David Carlton
carlton@kealia.com