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

[Jordan Rose via cfe-commits]

> On Nov 2, 2016, at 15:48, Richard Smith <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.)

We do still see this issue for fields of type ’T’, but that seemed trickier to deal with. I might have been overthinking it, though.

Jordan

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