<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Nov 2, 2016 at 2:34 PM, Jordan Rose via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><span class=""><blockquote type="cite"><div>On Nov 2, 2016, at 14:31, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:</div><br class="m_-4883782072324447123Apple-interchange-newline"><div><div dir="auto"><div><div class="gmail_extra"><div class="gmail_quote">On 2 Nov 2016 1:53 pm, "Jordan Rose via cfe-commits" <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br type="attribution"><blockquote class="m_-4883782072324447123quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: jrose<br>
Date: Wed Nov 2 15:44:07 2016<br>
New Revision: 285856<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=285856&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=285856&view=rev</a><br>
Log:<br>
Don't require nullability on template parameters in typedefs.<br>
<br>
Previously the following code would warn on the use of "T":<br>
<br>
template <typename T><br>
struct X {<br>
typedef T *type;<br>
};<br>
<br>
...because nullability is /allowed/ on template parameters (because<br>
they could be pointers). (Actually putting nullability on this use of<br>
'T' will of course break if the argument is a non-pointer type.)<br></blockquote></div></div></div><div><br></div><div>This doesn't make any sense to me. Why would T need to be a pointer type for a nullability qualifier to be valid on a T*?</div></div></div></blockquote><div><br></div></span><div>Sorry, this is referring to the following change to the example:</div><div><br></div></div><blockquote style="margin:0 0 0 40px;border:none;padding:0px"><span class=""><div><div>template <typename T></div></div><div><div>struct X {</div></div></span><div><div> typedef T _Nullable *type;</div></div><div><div>};</div></div></blockquote><div><div><br></div><div>This is legal, but of course `X<int>` then produces an error. So we want to accept nullability in this position (in case T is implicitly required to be a pointer type by the definition of X) but not warn when it’s missing (in case it isn’t).</div></div></div></blockquote><div><br></div><div>Oh, I see. Your testcase is very confusing, though, since it wraps the problematic use of T in a completely-unrelated pointer type. The actual problem being fixed is much more obvious in a case like this:</div><div><br></div><div>int *_Nullable p;</div><div>template<typename T> struct X {</div><div> T t; // warns without your fix</div><div>};</div><div><br></div><div>It'd be easier on future readers of this code to use the more obvious test case here.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><span class="HOEnZb"><font color="#888888"><div>Jordan</div></font></span><div><div class="h5"><br><blockquote type="cite"><div><div dir="auto"><div><br></div><div><div class="gmail_extra"><div class="gmail_quote"><blockquote class="m_-4883782072324447123quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This fix doesn't handle the case where a template parameter is used<br>
/outside/ of a typedef. That seems trickier, especially in parameter<br>
position.<br>
<br>
Modified:<br>
cfe/trunk/lib/Sema/SemaType.cp<wbr>p<br>
cfe/trunk/test/SemaObjCXX/Inpu<wbr>ts/nullability-consistency-1.h<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaType.cp<wbr>p<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=285856&r1=285855&r2=285856&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/lib/Sema/SemaT<wbr>ype.cpp?rev=285856&r1=285855&<wbr>r2=285856&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Sema/SemaType.cp<wbr>p (original)<br>
+++ cfe/trunk/lib/Sema/SemaType.cp<wbr>p Wed Nov 2 15:44:07 2016<br>
@@ -3600,7 +3600,17 @@ static TypeSourceInfo *GetFullTypeForDec<br>
// inner pointers.<br>
<wbr>complainAboutMissingNullabilit<wbr>y = CAMN_InnerPointers;<br>
<br>
- if (T->canHaveNullability() && !T->getNullability(S.Context)) {<br>
+ auto isDependentNonPointerType = [](QualType T) -> bool {<br>
+ // Note: This is intended to be the same check as Type::canHaveNullability<br>
+ // except with all of the ambiguous cases being treated as 'false' rather<br>
+ // than 'true'.<br>
+ return T->isDependentType() && !T->isAnyPointerType() &&<br>
+ !T->isBlockPointerType() && !T->isMemberPointerType();<br>
+ };<br>
+<br>
+ if (T->canHaveNullability() && !T->getNullability(S.Context) &&<br>
+ !isDependentNonPointerType(T)) {<br>
+ // Note that we allow but don't require nullability on dependent types.<br>
++NumPointersRemaining;<br>
}<br>
<br>
<br>
Modified: cfe/trunk/test/SemaObjCXX/Inpu<wbr>ts/nullability-consistency-1.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-1.h?rev=285856&r1=285855&r2=285856&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/test/SemaObjCX<wbr>X/Inputs/nullability-consisten<wbr>cy-1.h?rev=285856&r1=285855&<wbr>r2=285856&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/SemaObjCXX/Inpu<wbr>ts/nullability-consistency-1.h (original)<br>
+++ cfe/trunk/test/SemaObjCXX/Inpu<wbr>ts/nullability-consistency-1.h Wed Nov 2 15:44:07 2016<br>
@@ -13,5 +13,13 @@ class X {<br>
int X:: *memptr; // expected-warning{{member pointer is missing a nullability type specifier}}<br>
};<br>
<br>
+template <typename T><br>
+struct Typedefs {<br>
+ typedef T *Base; // no-warning<br>
+ typedef Base *type; // expected-warning{{pointer is missing a nullability type specifier}}<br>
+};<br>
+<br>
+Typedefs<int> xx;<br>
+Typedefs<void *> yy;<br>
<br>
<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div></div>
</div></blockquote></div></div></div><br></div><br>______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div></div>