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