[PATCH] D14938: Fix implicit conversion from _Nullable to _Nonnull warnings in C++
George Burgess IV via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 14 14:03:17 PST 2015
> Are there already tests in place for the C version? I assume so, but
> please double check.
Looks to me like the warning in the example is tested in C. Added 2
other cases (assignment + passing a _Nullable variable as a
_Nonnull argument) for good measure.
> LGTM with a couple of nits, below.
All done. Thanks for the review! :)
On Mon, Dec 14, 2015 at 1:20 PM, Justin Bogner <mail at justinbogner.com>
wrote:
> George Burgess IV via cfe-commits <cfe-commits at lists.llvm.org> writes:
> > george.burgess.iv created this revision.
> > george.burgess.iv added a reviewer: doug.gregor.
> > george.burgess.iv added subscribers: cfe-commits, doug.gregor.
> >
> > Currently, code like this compiles cleanly in C++, but with warnings
> > (as it should) in C:
>
> Are there already tests in place for the C version? I assume so, but
> please double check.
>
> >
> > ```
> > int foo() {
> > void *_Nullable p;
> > void *_Nonnull n = p;
> > }
> > ```
> >
> > This patch makes us emit the appropriate warnings in C++.
>
> LGTM with a couple of nits, below.
>
> > @doug.gregor: `arc` said you would be best to review this; if you're
> > unable to, I'm happy to ping others. :)
> >
> > http://reviews.llvm.org/D14938
> >
> > Files:
> > include/clang/Sema/Sema.h
> > lib/Sema/Sema.cpp
> > lib/Sema/SemaExpr.cpp
> > lib/Sema/SemaExprCXX.cpp
> > test/SemaCXX/nullability.cpp
> >
> > Index: test/SemaCXX/nullability.cpp
> > ===================================================================
> > --- test/SemaCXX/nullability.cpp
> > +++ test/SemaCXX/nullability.cpp
> > @@ -1,4 +1,4 @@
> > -// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wno-nullability-declspec
> %s -verify
> > +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wno-nullability-declspec
> %s -verify -Wnullable-to-nonnull-conversion
> >
> > #if __has_feature(nullability)
> > #else
> > @@ -67,3 +67,33 @@
> > }
> >
> > template void
> test_accepts_nonnull_null_pointer_literal_template<&accepts_nonnull_4>();
> // expected-note{{instantiation of function template specialization}}
> > +
> > +void TakeNonnull(void *_Nonnull);
> > +// Check different forms of assignment to a nonull type from a nullable
> one.
> > +void AssignAndInitNonNull() {
> > + void *_Nullable nullable;
> > + void *_Nonnull p(nullable); // expected-warning{{implicit conversion
> from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void
> * _Nonnull'}}
> > + void *_Nonnull p2{nullable}; // expected-warning{{implicit conversion
> from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void
> * _Nonnull'}}
> > + void *_Nonnull p3 = {nullable}; // expected-warning{{implicit
> conversion from nullable pointer 'void * _Nullable' to non-nullable pointer
> type 'void * _Nonnull'}}
> > + void *_Nonnull p4 = nullable; // expected-warning{{implicit
> conversion from nullable pointer 'void * _Nullable' to non-nullable pointer
> type 'void * _Nonnull'}}
> > + void *_Nonnull nonnull;
> > + nonnull = nullable; // expected-warning{{implicit conversion from
> nullable pointer 'void * _Nullable' to non-nullable pointer type 'void *
> _Nonnull'}}
> > + nonnull = {nullable}; // expected-warning{{implicit conversion from
> nullable pointer 'void * _Nullable' to non-nullable pointer type 'void *
> _Nonnull'}}
> > +
> > + TakeNonnull(nullable); //expected-warning{{implicit conversion from
> nullable pointer 'void * _Nullable' to non-nullable pointer type 'void *
> _Nonnull}}
> > + TakeNonnull(nonnull); // OK
> > +}
> > +
> > +void *_Nullable ReturnNullable();
> > +
> > +void AssignAndInitNonNullFromFn() {
> > + void *_Nonnull p(ReturnNullable()); // expected-warning{{implicit
> conversion from nullable pointer 'void * _Nullable' to non-nullable pointer
> type 'void * _Nonnull'}}
> > + void *_Nonnull p2{ReturnNullable()}; // expected-warning{{implicit
> conversion from nullable pointer 'void * _Nullable' to non-nullable pointer
> type 'void * _Nonnull'}}
> > + void *_Nonnull p3 = {ReturnNullable()}; // expected-warning{{implicit
> conversion from nullable pointer 'void * _Nullable' to non-nullable pointer
> type 'void * _Nonnull'}}
> > + void *_Nonnull p4 = ReturnNullable(); // expected-warning{{implicit
> conversion from nullable pointer 'void * _Nullable' to non-nullable pointer
> type 'void * _Nonnull'}}
> > + void *_Nonnull nonnull;
> > + nonnull = ReturnNullable(); // expected-warning{{implicit conversion
> from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void
> * _Nonnull'}}
> > + nonnull = {ReturnNullable()}; // expected-warning{{implicit
> conversion from nullable pointer 'void * _Nullable' to non-nullable pointer
> type 'void * _Nonnull'}}
> > +
> > + TakeNonnull(ReturnNullable()); //expected-warning{{implicit
> conversion from nullable pointer 'void * _Nullable' to non-nullable pointer
> type 'void * _Nonnull}}
> > +}
> > Index: lib/Sema/SemaExprCXX.cpp
> > ===================================================================
> > --- lib/Sema/SemaExprCXX.cpp
> > +++ lib/Sema/SemaExprCXX.cpp
> > @@ -3118,6 +3118,7 @@
> > ToType = ToAtomic->getValueType();
> > }
> >
> > + QualType InitialFromType = FromType;
> > // Perform the first implicit conversion.
> > switch (SCS.First) {
> > case ICK_Identity:
> > @@ -3488,6 +3489,12 @@
> > VK_RValue, nullptr, CCK).get();
> > }
> >
> > + // If this conversion sequence succeeded and involved implicitly
> converting a
> > + // _Nullable type to a _Nonnull one, complain.
> > + if (CCK == CCK_ImplicitConversion)
> > + diagnoseNullableToNonnullConversion(ToType, InitialFromType,
> > + From->getLocStart());
> > +
> > return From;
> > }
> >
> > Index: lib/Sema/SemaExpr.cpp
> > ===================================================================
> > --- lib/Sema/SemaExpr.cpp
> > +++ lib/Sema/SemaExpr.cpp
> > @@ -11823,8 +11823,8 @@
> >
> > switch (ConvTy) {
> > case Compatible:
> > - DiagnoseAssignmentEnum(DstType, SrcType, SrcExpr);
> > - return false;
> > + DiagnoseAssignmentEnum(DstType, SrcType, SrcExpr);
> > + return false;
>
> This looks unrelated / please commit formatting cleanup separately.
>
> >
> > case PointerToInt:
> > DiagKind = diag::ext_typecheck_convert_pointer_int;
> > Index: lib/Sema/Sema.cpp
> > ===================================================================
> > --- lib/Sema/Sema.cpp
> > +++ lib/Sema/Sema.cpp
> > @@ -349,6 +349,16 @@
> > AnalysisWarnings.PrintStats();
> > }
> >
> > +void Sema::diagnoseNullableToNonnullConversion(QualType DstType,
> > + QualType SrcType,
> > + SourceLocation Loc) {
> > + if (auto ExprNullability = SrcType->getNullability(Context))
> > + if (*ExprNullability == NullabilityKind::Nullable)
> > + if (auto TypeNullability = DstType->getNullability(Context))
> > + if (*TypeNullability == NullabilityKind::NonNull)
> > + Diag(Loc, diag::warn_nullability_lost) << SrcType << DstType;
> > +}
>
> This might be more readable with an early return or two. If it doesn't
> seem any more readable to you it's fine as is though.
>
> > +
> > /// ImpCastExprToType - If Expr is not of type 'Type', insert an
> implicit cast.
> > /// If there is already an implicit cast, merge into the existing one.
> > /// The result is of the given category.
> > @@ -372,18 +382,7 @@
> > assert((VK == VK_RValue || !E->isRValue()) && "can't cast rvalue to
> lvalue");
> > #endif
> >
> > - // Check whether we're implicitly casting from a nullable type to a
> nonnull
> > - // type.
> > - if (auto exprNullability = E->getType()->getNullability(Context)) {
> > - if (*exprNullability == NullabilityKind::Nullable) {
> > - if (auto typeNullability = Ty->getNullability(Context)) {
> > - if (*typeNullability == NullabilityKind::NonNull) {
> > - Diag(E->getLocStart(), diag::warn_nullability_lost)
> > - << E->getType() << Ty;
> > - }
> > - }
> > - }
> > - }
> > + diagnoseNullableToNonnullConversion(Ty, E->getType(),
> E->getLocStart());
> >
> > QualType ExprTy = Context.getCanonicalType(E->getType());
> > QualType TypeTy = Context.getCanonicalType(Ty);
> > Index: include/clang/Sema/Sema.h
> > ===================================================================
> > --- include/clang/Sema/Sema.h
> > +++ include/clang/Sema/Sema.h
> > @@ -3505,6 +3505,11 @@
> > void DiagnoseSelfMove(const Expr *LHSExpr, const Expr *RHSExpr,
> > SourceLocation OpLoc);
> >
> > + /// \brief Warn if we're implicitly casting from a _Nullable pointer
> type to a
> > + /// _Nonnull one.
> > + void diagnoseNullableToNonnullConversion(QualType DstType, QualType
> SrcType,
> > + SourceLocation Loc);
> > +
> > ParsingDeclState PushParsingDeclaration(sema::DelayedDiagnosticPool
> &pool) {
> > return DelayedDiagnostics.push(pool);
> > }
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151214/74293ffe/attachment-0001.html>
More information about the cfe-commits
mailing list