[cfe-commits] r111163 - in /cfe/trunk: lib/Parse/ParseDecl.cpp test/Parser/cxx-decl.cpp

[Douglas Gregor]

On Aug 16, 2010, at 10:58 AM, Fariborz Jahanian wrote:

> Author: fjahanian
> Date: Mon Aug 16 12:58:53 2010
> New Revision: 111163
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=111163&view=rev
> Log:
> Fix a crash when parsing malformed out-of-line member function 
> definition. radar 8307865.
> 
> Modified:
>    cfe/trunk/lib/Parse/ParseDecl.cpp
>    cfe/trunk/test/Parser/cxx-decl.cpp
> 
> Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=111163&r1=111162&r2=111163&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseDecl.cpp Mon Aug 16 12:58:53 2010
> @@ -2727,7 +2727,10 @@
>       if (Actions.ShouldEnterDeclaratorScope(getCurScope(), D.getCXXScopeSpec()))
>         // Change the declaration context for name lookup, until this function
>         // is exited (and the declarator has been parsed).
> -        DeclScopeObj.EnterDeclaratorScope();
> +        // If there was an error parsing parenthesized declarator, declarator
> +        // scope may have been enterred before. Don't do it again.
> +        if (!D.isInvalidType())
> +          DeclScopeObj.EnterDeclaratorScope();
>     }

A couple comments on this one...

Is there any reason not to just fold the isInvalidType() check into the "if" statement above? 

Also, I see a few other places where we EnterDeclaratorScope() within ParseDecl.cpp. Do those need to be updated?

Finally, it seems a little suspicious to me that we're checking for an invalid *type* here, when entering the declarator scope only deals with the CXXScopeSpec. Should the check look at the CXXScopeSpec, or is there some relationship between an invalid CXXScopeSpec and an invalid declarator type?

	- Doug