George Burgess IV via cfe-commits <cfe-commits@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@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
