<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Dec 14, 2015 at 10:39 AM, Justin Bogner <span dir="ltr"><<a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</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><div>Akira Hatanaka via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> writes:<br>
> ahatanak created this revision.<br>
> ahatanak added a subscriber: cfe-commits.<br>
><br>
> This patch fixes a crash that occurs when __kindof is incorrectly used<br>
> in the type parameter list of an interface. The crash occurs because<br>
> ObjCTypeParamList::back() is called in checkTypeParamListConsistency<br>
> on an empty list:<br>
><br>
> 00762 diagLoc = S.getLocForEndOfToken(newTypeParams->back()->getLocEnd());<br>
><br>
> <a href="http://reviews.llvm.org/D15463" rel="noreferrer" target="_blank">http://reviews.llvm.org/D15463</a><br>
><br>
> Files:<br>
> lib/Parse/ParseObjc.cpp<br>
> test/SemaObjC/kindof.m<br>
><br>
> Index: test/SemaObjC/kindof.m<br>
> ===================================================================<br>
> --- test/SemaObjC/kindof.m<br>
> +++ test/SemaObjC/kindof.m<br>
> @@ -302,3 +302,13 @@<br>
> void processCopyable(__typeof(getSomeCopyable()) string);<br>
> processCopyable(0); // expected-warning{{null passed to a callee that requires a non-null argument}}<br>
> }<br>
> +<br>
> +// __kinddof cannot be used in parameter list.<br>
> +@interface Array1<T> : NSObject<br>
> +@end<br>
> +<br>
> +@interface I1 : NSObject<br>
> +@end<br>
> +<br>
> +@interface Array1<__kindof I1*>(extensions) // // expected-error{{expected type parameter name}}<br>
> +@end<br>
> Index: lib/Parse/ParseObjc.cpp<br>
> ===================================================================<br>
> --- lib/Parse/ParseObjc.cpp<br>
> +++ lib/Parse/ParseObjc.cpp<br>
> @@ -603,7 +603,7 @@<br>
> // whether there are any protocol references.<br>
> lAngleLoc = SourceLocation();<br>
> rAngleLoc = SourceLocation();<br>
> - return list;<br>
> + return invalid ? nullptr : list;<br>
<br>
</div></div>This looks a bit suspicious to me. Since `invalid` is set *way* earlier<br>
in the function, it seems like we should be able to return earlier if<br>
this is correct, and not even call actOnObjCTypeParamList. OTOH, are<br>
there cases where `invalid == true` but list is non-empty? If so, are we<br>
doing the right thing when that happens?<br>
<br></blockquote><div><br></div><div>I'm assuming you meant it should return earlier if this is *incorrect* (but let me know if I misunderstood your comment).</div><div><br></div><div>The only downside to returning nullptr before actOnObjCTypeParamList is called in line 595 is that it won't print the diagnostics that are printed in actOnObjCTypeParamList. For example, if the following interface were compiled,</div><div><br></div><div>@interface I1<T, T, __kindof I2*> NSObject</div><div>@end</div><div> </div><div>actOnObjCTypeParamList could tell the user that parameter "T" was redeclared in addition to printing the error message about __kindof. So if we return before it's called, we won't see that error message.</div><div><br></div><div>When the interface declaration of category extensions1 in the following piece of code is compiled, invalid is set to true and the list is not empty (it contains the first two parameters).</div><div><br></div><div><div><div>@interface I1<T1, T2, T3> NSObject</div><div>@end</div></div></div><div><br></div><div><div>@interface I1<T, T, __kindof I2*> (extensions1)</div><div>@end</div></div><div><br></div><div>
<p class=""><span class=""><b>test3.m:6:23: </b></span><span class=""><b>error: </b></span><span class=""><b>expected type parameter name</b></span></p>
<p class=""><span class="">@interface I6<T1, T2, __kindof I2*> (extensions1)</span></p>
<p class=""><span class=""><b> ^</b></span></p>
<p class=""><span class=""><b>test3.m:6:21: </b></span><span class=""><b>error: </b></span><span class=""><b>category has too few type parameters (expected 3, have 2)</b></span></p>
<p class=""><span class="">@interface I6<T1, T2, __kindof I2*> (extensions1)</span></p></div><div>It looks like it's doing the right thing, but the second diagnostic doesn't seem necessary to me.</div><div><br></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">
> }<br>
><br>
> /// Parse an objc-type-parameter-list.<br>
><br>
><br>
</blockquote></div><br></div></div>