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

[Eli Friedman]

On Fri, Jun 20, 2008 at 2:09 PM, Argiris Kirtzidis <akyrtzi at gmail.com> wrote:
>> In Parser::ParseCXXClassMemberDeclaration:
>> You mention the pure specifier in the comments, but it looks like you
>> forgot to actually implement it (or at least put in a FIXME where it
>> should be implemented).
>>
>
> I've put a [TODO] for the pure specifier on the top of
> ParseCXXClassMemberDeclaration. And I've put
>
>>        // Defer all other checks to Sema::AddInitializerToDecl.
>
> which is where it is supposed to be handled. Shall I add a more specific
> comment for pure specifier ?

Nevermind; I was thinking it might be difficult to handle this from
Sema because the AST doesn't know the difference between different
ways of writing 0, but I don't think it'll be an issue.

>> In Parser::TryParseFunctionOrCXXMethodDef:
>> +      Tok.is(tok::colon) ||           // int X():  -> not a function def
>> Hmm?  Doesn't a colon guarantee that this is a function definition
>> with a ctor-initializer?  Or are you planning on handling this case
>> some other way?
>>
>
> I think that constructors will be handled in some other way; as I see it,
> this method will be used on declarators with declaration specifiers.
> If it turns out that it'd be useful for constructors to go through this
> method, we can always change it later.

Okay, makes sense.

> You are referring to
>  assert(CurFunctionDecl == 0 && "Confused parsing.");
> right ?

Yes.

A couple more potential issues that I found while re-reading the patch:
+    bool isInstField = (DS.getStorageClassSpec() ==
DeclSpec::SCS_unspecified &&
+                        !DeclaratorInfo.isFunctionDeclarator());

This check isn't reliable. Simple example: "typedef int func(); class
C {func a,b;};".

+  typedef std::list<LexedInlineMethod> LexedInlineMethodsTy;
+  /// LexedInlineMethods - During parsing of a C++ class, its inline method
+  /// definitions and the inline method definitions of its nested classes are
+  /// lexed and stored here.
+  LexedInlineMethodsTy LexedInlineMethods;
[...]
+  if (!(CurScope->getFlags() & Scope::CXXClassScope)) {
+    // We are no longer inside a C++ class. This class and its nested classes
+    // are complete and we can parse the lexed inline method definitions.
+    for (LexedInlineMethodsTy::iterator I = LexedInlineMethods.begin();
+         I != LexedInlineMethods.end(); ++I) {

This isn't quite right; consider the following:
class X {

  int outer() {
   static int b;
   class C {
     public:
     int inner() {
       return b;
     }
   } r;
   return r.inner();
  };

  int other() {return b;}

  int x;
};

With the current code, at least according to my reading, first X gets
parsed, then we start parsing outer, then we finish parsing class C,
then we attempt to start parsing outer again, then bad things happen.

I'd suggest allocating LexedInlineMethods in
ParseCXXMemberSpecification, rather than making it a member of Parser.

-Eli