On Wed, Nov 2, 2016 at 3:51 PM, Jordan Rose via cfe-commits < cfe-commits@lists.llvm.org> wrote:
> > On Nov 2, 2016, at 15:48, Richard Smith <rich...@metafoo.co.uk> wrote: > > On Wed, Nov 2, 2016 at 2:34 PM, Jordan Rose via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> >> On Nov 2, 2016, at 14:31, Richard Smith <rich...@metafoo.co.uk> wrote: >> >> On 2 Nov 2016 1:53 pm, "Jordan Rose via cfe-commits" < >> cfe-commits@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 >> 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. > 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 > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
