r285856 - Don't require nullability on template parameters in typedefs.

[Jordan Rose via cfe-commits]

> On Nov 2, 2016, at 16:20, Richard Smith <richard at metafoo.co.uk> wrote:
> 
> On Wed, Nov 2, 2016 at 3:51 PM, Jordan Rose via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
> 
>> On Nov 2, 2016, at 15:48, Richard Smith <richard at metafoo.co.uk <mailto:richard at metafoo.co.uk>> wrote:
>> 
>> On Wed, Nov 2, 2016 at 2:34 PM, Jordan Rose via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
>> 
>>> On Nov 2, 2016, at 14:31, Richard Smith <richard at metafoo.co.uk <mailto:richard at metafoo.co.uk>> wrote:
>>> 
>>> On 2 Nov 2016 1:53 pm, "Jordan Rose via cfe-commits" <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
>>> Author: jrose
>>> Date: Wed Nov  2 15:44:07 2016
>>> New Revision: 285856
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=285856&view=rev <http://llvm.org/viewvc/llvm-project?rev=285856&view=rev>
>>> Log:
>>> Don't require nullability on template parameters in typedefs.
>>> 
>>> Previously the following code would warn on the use of "T":
>>> 
>>>   template <typename T>
>>>   struct X {
>>>     typedef T *type;
>>>   };
>>> 
>>> ...because nullability is /allowed/ on template parameters (because
>>> they could be pointers). (Actually putting nullability on this use of
>>> 'T' will of course break if the argument is a non-pointer type.)
>>> 
>>> 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*?
>> 
>> Sorry, this is referring to the following change to the example:
>> 
>> template <typename T>
>> struct X {
>>   typedef T _Nullable *type;
>> };
>> 
>> 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).
>> 
>> 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:
>> 
>> int *_Nullable p;
>> template<typename T> struct X {
>>   T t; // warns without your fix
>> };
>> 
>> It'd be easier on future readers of this code to use the more obvious test case here.
> 
> Ah, that case doesn’t actually trigger the issue, because a typedef doesn’t require nullability on its outermost pointer type. (It’s assumed that the use site may need to make some uses of the typedef nullable and some non-nullable.)
> 
> The testcase I gave above definitely produces a bogus warning before your patch. I've not actually checked whether the patch fixes it, though.
>  

Oops, sorry, I misread it thinking it still said ‘typedef’. Yes, that does warn, and no, this patch does not fix it.

Jordan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161102/64ed0560/attachment-0001.html>