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

[Argiris Kirtzidis]

Hi Chris,

Chris Lattner wrote:
>
> +  /// LexedMethodStacks - During parsing of a top (non-nested) C++ 
> class, its
> +  /// inline method definitions and the inline method definitions of 
> its nested
> +  /// classes are lexed and stored here.
> +  /// A new lexed methods stack is pushed for local classes in inline 
> methods.
> +  std::stack< std::stack<LexedMethod> > LexedMethodStacks;
>
> Out of curiousity, why use a std::stack instead of std::deque or 
> std::vector?

About the outer stack, it's the last "MethodTokensForOneClass" pushed 
that is going to be used for adding lexed methods, so a stack seems 
appropriate.
About the inner stack, there won't be a difference whether it's a stack, 
list or deque, although std::vector might not be as appropriate since 
there's no need to have LexedMethods in contiguous memory.
Since a std::stack was already brought in for the outer stack, I figured 
I could use it again to avoid bringing in an additional container.

>
> For clarity, it might be better to make a typedef for the inner stack, 
> something like:
>
>   typedef std::stack<LexedMethod> MethodTokensForOneClass;
>
> or something?  Use of that typedef would make the interfaces a bit 
> more clear.
>
> +  void PushLexedMethodStack() {
> +  void PopLexedMethodStack() { LexedMethodStacks.pop(); }
>
> These really push one 'nested class' worth of methods, right?  It 
> might be better to name these "PushClassStack" or something.

Ok to both suggestions.
Just a sidenote, I think these containers and methods will need 
refactoring to accomodate other types of "delayed parsing", like default 
parameters. So they are subject to change; probably a new class will be 
added for all this.

>
>
>
> +++ lib/Parse/ParseCXXInlineMethods.cpp    (revision 0)
>
> A new file is a great idea.
>
>
> + Parser::ParseCXXInlineMethodDef(AccessSpecifier AS, Declarator &D) {
> +  assert(D.getTypeObject(0).Kind == DeclaratorChunk::Function &&
> +         "This isn't a function declarator!");
> +
> +  DeclTy *FnD = Actions.ActOnCXXMemberDeclarator(CurScope, AS, D, 0, 
> 0, 0);
> +
> +  // We should have an opening brace now.
> +  if (Tok.isNot(tok::l_brace)) {
>
> The only caller of this knows that the current token is l_brace, so 
> this can be an assert.

Ok.

>
>
>
> In Parser::ConsumeAndStoreUntil, do you think it would make sense to 
> specialize it for T=r_brace, or do you expect other clients?

This method calls itself recursively for braces and parentheses.


-Argiris