[cfe-dev] [PATCH]: Parsing for C++ classes

[Chris Lattner]

On Jun 20, 2008, at 2:09 PM, Argiris Kirtzidis wrote:
>> It looks like you're using this for "method body eating".  It would  
>> also be reasonable to have a specialized version of this that just  
>> counts braces/parens etc.  I consider SkipUntil to really be part  
>> of the diagnostic machinery... having it be used by the parser is a  
>> bit strange to me, and could make future extensions to it more  
>> difficult.
>
> I don't quite see how the specialized version will be different from  
> SkipUntil. You still need to consider braces,parens,brackets,  
> strings etc. Basically the implementation of SkipUntil will be copied.
> Is it ok to duplicate the code for this ?

Okay, you're right.  I think using a "function pointer + void*" is the  
right tradeoff.

>> +  // C++ 9.2p6: A member shall not be declared to have automatic  
>> storage
>> +  // duration (auto, register) or with the extern storage-class- 
>> specifier.
>> +  switch (DS.getStorageClassSpec()) {
>> +    case DeclSpec::SCS_unspecified:
>>
>> Should this code live in parser or in sema?  It seems cleaner to  
>> keep the 'analysis' code in sema as much as possible.  I know we  
>> aren't necessarily clean about this everywhere, but I think it  
>> would be useful to avoid this if possible.
>
> Ok, I was under the impression that if the parser has the necessary  
> information to determine an error, it should emit the error.
> Is the parser supposed to defer to Sema as many checks as possible?

There is no one right answer unfortunately.  When I initially started  
on clang, that is the direction I went because I didn't have sema and  
wanted to implement some of these checks.

Advantages of doing everything in sema:

1. All semantic checking is in one place.
2. It is much more obvious when checks are duplicated.  Right now, it  
is easy to do something in sema that is already covered by the parser,  
or are only covered in some cases.
3. Moving sema checks into sema keeps the parser proper clean and  
easier to understand.
4. Clients that don't care about sema (e.g. things that just parse to  
get function names) get a small amount of "error tolerance" if the  
parser is only strict about stuff that affects the parse.

Advantages of doing it in parser:

1. Every action implementation doesn't need to reimplement the checks.


While we do checks in both parser and sema, I think we should migrate  
to doing everything in sema where possible.  Seem reasonable?

>> +      // Append the current token at the end of the new token  
>> stream so that it
>> +      // doesn't get lost.
>> +      I->Toks.push_back(Tok);
>> +      PP.EnterTokenStream(&I->Toks.front(), I->Toks.size(), true,  
>> false);
>>
>> Very nice!  If there are multiple methods, should this push Tok for  
>> all of them?
>
> Yes, after the method is parsed, the current token is the previously  
> pushed token, and it needs to be pushed again.

Ok.

>
>
>>
>> +Parser::ExprResult Parser::ParseCXXThis() {
>> +  assert(Tok.is(tok::kw_this) && "Not 'this'!");
>> +  SourceLocation ThisLoc = ConsumeToken();
>> +  if (CurScope->getFnParent() == 0) {
>> +    Diag(ThisLoc, diag::err_invalid_this_at_top_level);
>> +    return ExprResult(true);
>> +  }
>>
>> Should this also check for static methods?  What about C  
>> functions?  Alternatively, does sema handle these checks?  If so,  
>> should Sema handle all the checks?
>
> Static methods and C functions are checked on Sema since the parser  
> don't have this kind of information.
> This applies to my previous question about where should the checks  
> be handled.

I think this is a great example of something that would be cleaner/ 
more obvious in sema, because it could handle all the cases uniformly  
in one place.

-Chris