On Fri, Jan 16, 2015 at 5:55 PM, Richard Smith <[email protected]> wrote:
> On Fri, Jan 16, 2015 at 11:34 AM, Nico Weber <[email protected]> wrote: > > Author: nico > > Date: Fri Jan 16 13:34:13 2015 > > New Revision: 226306 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=226306&view=rev > > Log: > > Don't crash if a declarator in a friend decl doesn't have a name. > > > > There was already an explicit check for that for the first decl. Move > that > > to a different place so that it's called for the following decls too. > Also > > don't randomly set the BitfieldSize ExprResult to true (this sets a > pointer to > > true internally). > > > > Found by SLi's bot. > > > > Modified: > > cfe/trunk/include/clang/Parse/Parser.h > > cfe/trunk/lib/Parse/ParseDeclCXX.cpp > > cfe/trunk/test/CXX/class/class.friend/p1.cpp > > > > Modified: cfe/trunk/include/clang/Parse/Parser.h > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=226306&r1=226305&r2=226306&view=diff > > > ============================================================================== > > --- cfe/trunk/include/clang/Parse/Parser.h (original) > > +++ cfe/trunk/include/clang/Parse/Parser.h Fri Jan 16 13:34:13 2015 > > @@ -2294,7 +2294,7 @@ private: > > Decl *TagDecl); > > ExprResult ParseCXXMemberInitializer(Decl *D, bool IsFunction, > > SourceLocation &EqualLoc); > > - void ParseCXXMemberDeclaratorBeforeInitializer(Declarator > &DeclaratorInfo, > > + bool ParseCXXMemberDeclaratorBeforeInitializer(Declarator > &DeclaratorInfo, > > VirtSpecifiers &VS, > > ExprResult > &BitfieldSize, > > LateParsedAttrList > &LateAttrs); > > > > Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=226306&r1=226305&r2=226306&view=diff > > > ============================================================================== > > --- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original) > > +++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Fri Jan 16 13:34:13 2015 > > @@ -2019,7 +2019,7 @@ bool Parser::isCXX11FinalKeyword() const > > > > /// \brief Parse a C++ member-declarator up to, but not including, the > optional > > /// brace-or-equal-initializer or pure-specifier. > > -void Parser::ParseCXXMemberDeclaratorBeforeInitializer( > > +bool Parser::ParseCXXMemberDeclaratorBeforeInitializer( > > Declarator &DeclaratorInfo, VirtSpecifiers &VS, ExprResult > &BitfieldSize, > > LateParsedAttrList &LateParsedAttrs) { > > // member-declarator: > > @@ -2073,6 +2073,15 @@ void Parser::ParseCXXMemberDeclaratorBef > > } > > } > > } > > + > > + // If this has neither a name nor a bit width, something has gone > seriously > > + // wrong. Skip until the semi-colon or }. > > + if (!DeclaratorInfo.hasName() && BitfieldSize.isUnset()) { > > + // If so, skip until the semi-colon or a }. > > + SkipUntil(tok::r_brace, StopAtSemi | StopBeforeMatch); > > + return true; > > + } > > + return false; > > } > > > > /// ParseCXXClassMemberDeclaration - Parse a C++ class member > declaration. > > @@ -2298,14 +2307,8 @@ void Parser::ParseCXXClassMemberDeclarat > > bool ExpectSemi = true; > > > > // Parse the first declarator. > > - ParseCXXMemberDeclaratorBeforeInitializer(DeclaratorInfo, VS, > BitfieldSize, > > - LateParsedAttrs); > > - > > - // If this has neither a name nor a bit width, something has gone > seriously > > - // wrong. Skip until the semi-colon or }. > > - if (!DeclaratorInfo.hasName() && BitfieldSize.isUnset()) { > > - // If so, skip until the semi-colon or a }. > > - SkipUntil(tok::r_brace, StopAtSemi | StopBeforeMatch); > > + if (ParseCXXMemberDeclaratorBeforeInitializer( > > + DeclaratorInfo, VS, BitfieldSize, LateParsedAttrs)) { > > TryConsumeToken(tok::semi); > > return; > > } > > @@ -2530,7 +2533,7 @@ void Parser::ParseCXXClassMemberDeclarat > > // Parse the next declarator. > > DeclaratorInfo.clear(); > > VS.clear(); > > - BitfieldSize = true; > > This was previously setting BitfieldSize to ExprError() (that is, null > and invalid) and now sets it to ExprResult() (that is, null but > valid). Was that your intention? > Yes: The if above checks if (!DeclaratorInfo.hasName() && BitfieldSize.isUnset()), and isUnset() is implemented as `return !Invalid && !Val;`. Without this change, the error path wasn't taken. > > > + BitfieldSize = nullptr; > > Init = true; > > If you don't like the implicit conversion from bool to ExprResult (and > I don't blame you...) please fix this one too for local consistency. > I added an explicit constructor call and flipped this from true to false (from what I can tell, this shouldn't make a behavior difference for this one) in r226363. Thanks! > > > HasInitializer = false; > > DeclaratorInfo.setCommaLoc(CommaLoc); > > @@ -2538,8 +2541,9 @@ void Parser::ParseCXXClassMemberDeclarat > > // GNU attributes are allowed before the second and subsequent > declarator. > > MaybeParseGNUAttributes(DeclaratorInfo); > > > > - ParseCXXMemberDeclaratorBeforeInitializer(DeclaratorInfo, VS, > BitfieldSize, > > - LateParsedAttrs); > > + if (ParseCXXMemberDeclaratorBeforeInitializer( > > + DeclaratorInfo, VS, BitfieldSize, LateParsedAttrs)) > > + break; > > } > > > > if (ExpectSemi && > > > > Modified: cfe/trunk/test/CXX/class/class.friend/p1.cpp > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class/class.friend/p1.cpp?rev=226306&r1=226305&r2=226306&view=diff > > > ============================================================================== > > --- cfe/trunk/test/CXX/class/class.friend/p1.cpp (original) > > +++ cfe/trunk/test/CXX/class/class.friend/p1.cpp Fri Jan 16 13:34:13 2015 > > @@ -66,6 +66,10 @@ class A { > > class facet; > > friend class facet; // should not assert > > class facet {}; > > + > > + friend int Unknown::thing(); // expected-error {{use of undeclared > identifier}} > > + friend int friendfunc(), Unknown::thing(); // expected-error {{use of > undeclared identifier}} > > + friend int friendfunc(), Unknown::thing() : 4; // expected-error > {{use of undeclared identifier}} > > }; > > > > A::UndeclaredSoFar y; // expected-error {{no type named > 'UndeclaredSoFar' in 'A'}} > > > > > > _______________________________________________ > > cfe-commits mailing list > > [email protected] > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
