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. Jordan > > > This fix doesn't handle the case where a template parameter is used > /outside/ of a typedef. That seems trickier, especially in parameter > position. > > Modified: > cfe/trunk/lib/Sema/SemaType.cpp > cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-1.h > > Modified: cfe/trunk/lib/Sema/SemaType.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaT > ype.cpp?rev=285856&r1=285855&r2=285856&view=diff > ============================================================ > ================== > --- cfe/trunk/lib/Sema/SemaType.cpp (original) > +++ cfe/trunk/lib/Sema/SemaType.cpp Wed Nov 2 15:44:07 2016 > @@ -3600,7 +3600,17 @@ static TypeSourceInfo *GetFullTypeForDec > // inner pointers. > complainAboutMissingNullability = CAMN_InnerPointers; > > - if (T->canHaveNullability() && !T->getNullability(S.Context)) { > + auto isDependentNonPointerType = [](QualType T) -> bool { > + // Note: This is intended to be the same check as > Type::canHaveNullability > + // except with all of the ambiguous cases being treated as 'false' > rather > + // than 'true'. > + return T->isDependentType() && !T->isAnyPointerType() && > + !T->isBlockPointerType() && !T->isMemberPointerType(); > + }; > + > + if (T->canHaveNullability() && !T->getNullability(S.Context) && > + !isDependentNonPointerType(T)) { > + // Note that we allow but don't require nullability on dependent > types. > ++NumPointersRemaining; > } > > > Modified: cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-1.h > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCX > X/Inputs/nullability-consistency-1.h?rev=285856&r1=285855& > r2=285856&view=diff > ============================================================ > ================== > --- cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-1.h > (original) > +++ cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-1.h Wed Nov > 2 15:44:07 2016 > @@ -13,5 +13,13 @@ class X { > int X:: *memptr; // expected-warning{{member pointer is missing a > nullability type specifier}} > }; > > +template <typename T> > +struct Typedefs { > + typedef T *Base; // no-warning > + typedef Base *type; // expected-warning{{pointer is missing a > nullability type specifier}} > +}; > + > +Typedefs<int> xx; > +Typedefs<void *> yy; > > > > > _______________________________________________ > 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 > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
