From 768c84e6b75f36035b2f83616a0dc0e096941299 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Thu, 10 Nov 2016 23:28:30 +0000 Subject: [PATCH] Warn when 'assume_nonnull' infers nullability within an array. ...or within a reference. Both of these add an extra level of indirection that make us less certain that the pointer really was supposed to be non-nullable. However, changing the default behavior would be a breaking change, so we'll just make it a warning instead. Part of rdar://problem/25846421 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@286521 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticGroups.td | 1 + include/clang/Basic/DiagnosticSemaKinds.td | 5 ++ lib/Sema/SemaType.cpp | 57 ++++++++++++++-- test/FixIt/nullability.mm | 68 +++++++++++++++++++ .../nullability-consistency-arrays.mm | 12 ++-- 5 files changed, 130 insertions(+), 13 deletions(-) create mode 100644 test/FixIt/nullability.mm diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td index 4322048a41..35a3473f6a 100644 --- a/include/clang/Basic/DiagnosticGroups.td +++ b/include/clang/Basic/DiagnosticGroups.td @@ -277,6 +277,7 @@ def ModuleFileExtension : DiagGroup<"module-file-extension">; def NewlineEOF : DiagGroup<"newline-eof">; def Nullability : DiagGroup<"nullability">; def NullabilityDeclSpec : DiagGroup<"nullability-declspec">; +def NullabilityInferredOnNestedType : DiagGroup<"nullability-inferred-on-nested-type">; def NullableToNonNullConversion : DiagGroup<"nullable-to-nonnull-conversion">; def NullabilityCompletenessOnArrays : DiagGroup<"nullability-completeness-on-arrays">; def NullabilityCompleteness : DiagGroup<"nullability-completeness", diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index c4a234d9c4..4e7b664995 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -8737,6 +8737,11 @@ def warn_nullability_missing_array : Warning< "_Nullable, or _Null_unspecified)">, InGroup; +def warn_nullability_inferred_on_nested_type : Warning< + "inferring '_Nonnull' for pointer type within %select{array|reference}0 is " + "deprecated">, + InGroup; + def err_objc_type_arg_explicit_nullability : Error< "type argument %0 cannot explicitly specify nullability">; diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp index ec59b1235e..0ae53a13d0 100644 --- a/lib/Sema/SemaType.cpp +++ b/lib/Sema/SemaType.cpp @@ -3274,15 +3274,27 @@ namespace { // NSError** NSErrorPointerPointer, }; + + /// Describes a declarator chunk wrapping a pointer that marks inference as + /// unexpected. + // These values must be kept in sync with diagnostics. + enum class PointerWrappingDeclaratorKind { + /// Pointer is top-level. + None = -1, + /// Pointer is an array element. + Array = 0, + /// Pointer is the referent type of a C++ reference. + Reference = 1 + }; } // end anonymous namespace /// Classify the given declarator, whose type-specified is \c type, based on /// what kind of pointer it refers to. /// /// This is used to determine the default nullability. -static PointerDeclaratorKind classifyPointerDeclarator(Sema &S, - QualType type, - Declarator &declarator) { +static PointerDeclaratorKind +classifyPointerDeclarator(Sema &S, QualType type, Declarator &declarator, + PointerWrappingDeclaratorKind &wrappingKind) { unsigned numNormalPointers = 0; // For any dependent type, we consider it a non-pointer. @@ -3294,6 +3306,10 @@ static PointerDeclaratorKind classifyPointerDeclarator(Sema &S, DeclaratorChunk &chunk = declarator.getTypeObject(i); switch (chunk.Kind) { case DeclaratorChunk::Array: + if (numNormalPointers == 0) + wrappingKind = PointerWrappingDeclaratorKind::Array; + break; + case DeclaratorChunk::Function: case DeclaratorChunk::Pipe: break; @@ -3304,14 +3320,18 @@ static PointerDeclaratorKind classifyPointerDeclarator(Sema &S, : PointerDeclaratorKind::SingleLevelPointer; case DeclaratorChunk::Paren: + break; + case DeclaratorChunk::Reference: - continue; + if (numNormalPointers == 0) + wrappingKind = PointerWrappingDeclaratorKind::Reference; + break; case DeclaratorChunk::Pointer: ++numNormalPointers; if (numNormalPointers > 2) return PointerDeclaratorKind::MultiLevelPointer; - continue; + break; } } @@ -3657,6 +3677,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state, CAMN_Yes } complainAboutMissingNullability = CAMN_No; unsigned NumPointersRemaining = 0; + auto complainAboutInferringWithinChunk = PointerWrappingDeclaratorKind::None; if (IsTypedefName) { // For typedefs, we do not infer any nullability (the default), @@ -3725,11 +3746,12 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state, // fallthrough case Declarator::FileContext: - case Declarator::KNRTypeListContext: + case Declarator::KNRTypeListContext: { complainAboutMissingNullability = CAMN_Yes; // Nullability inference depends on the type and declarator. - switch (classifyPointerDeclarator(S, T, D)) { + auto wrappingKind = PointerWrappingDeclaratorKind::None; + switch (classifyPointerDeclarator(S, T, D, wrappingKind)) { case PointerDeclaratorKind::NonPointer: case PointerDeclaratorKind::MultiLevelPointer: // Cannot infer nullability. @@ -3738,6 +3760,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state, case PointerDeclaratorKind::SingleLevelPointer: // Infer _Nonnull if we are in an assumes-nonnull region. if (inAssumeNonNullRegion) { + complainAboutInferringWithinChunk = wrappingKind; inferNullability = NullabilityKind::NonNull; inferNullabilityCS = (context == Declarator::ObjCParameterContext || context == Declarator::ObjCResultContext); @@ -3778,6 +3801,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state, break; } break; + } case Declarator::ConversionIdContext: complainAboutMissingNullability = CAMN_Yes; @@ -3836,6 +3860,25 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state, ->setObjCDeclQualifier(ObjCDeclSpec::DQ_CSNullability); } + if (pointerLoc.isValid() && + complainAboutInferringWithinChunk != + PointerWrappingDeclaratorKind::None) { + SourceLocation fixItLoc = S.getLocForEndOfToken(pointerLoc); + StringRef insertionText = " _Nonnull "; + if (const char *nextChar = S.SourceMgr.getCharacterData(fixItLoc)) { + if (isWhitespace(*nextChar)) { + insertionText = insertionText.drop_back(); + } else if (!isIdentifierBody(nextChar[0], /*allow dollar*/true) && + !isIdentifierBody(nextChar[-1], /*allow dollar*/true)) { + insertionText = insertionText.drop_back().drop_front(); + } + } + + S.Diag(pointerLoc, diag::warn_nullability_inferred_on_nested_type) + << static_cast(complainAboutInferringWithinChunk) + << FixItHint::CreateInsertion(fixItLoc, insertionText); + } + if (inferNullabilityInnerOnly) inferNullabilityInnerOnlyComplete = true; return nullabilityAttr; diff --git a/test/FixIt/nullability.mm b/test/FixIt/nullability.mm new file mode 100644 index 0000000000..244906601f --- /dev/null +++ b/test/FixIt/nullability.mm @@ -0,0 +1,68 @@ +// RUN: %clang_cc1 -fsyntax-only -fblocks -std=gnu++11 -verify %s +// RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -fblocks -std=gnu++11 %s 2>&1 | FileCheck %s + +#pragma clang assume_nonnull begin + +extern void *array[2]; // expected-warning {{inferring '_Nonnull' for pointer type within array is deprecated}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:14-[[@LINE-1]]:14}:" _Nonnull " + +extern void* array2[2]; // expected-warning {{inferring '_Nonnull' for pointer type within array is deprecated}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:13}:" _Nonnull" + +extern void *nestedArray[2][3]; // expected-warning {{inferring '_Nonnull' for pointer type within array is deprecated}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:14-[[@LINE-1]]:14}:" _Nonnull " + + +typedef const void *CFTypeRef; + +extern CFTypeRef typedefArray[2]; // expected-warning {{inferring '_Nonnull' for pointer type within array is deprecated}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:17-[[@LINE-1]]:17}:" _Nonnull" + + +extern void *&ref; // expected-warning {{inferring '_Nonnull' for pointer type within reference is deprecated}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:14-[[@LINE-1]]:14}:"_Nonnull" + +extern void * &ref2; // expected-warning {{inferring '_Nonnull' for pointer type within reference is deprecated}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:14-[[@LINE-1]]:14}:" _Nonnull" + +extern void *&&ref3; // expected-warning {{inferring '_Nonnull' for pointer type within reference is deprecated}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:14-[[@LINE-1]]:14}:"_Nonnull" + +extern void * &&ref4; // expected-warning {{inferring '_Nonnull' for pointer type within reference is deprecated}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:14-[[@LINE-1]]:14}:" _Nonnull" + +extern void *(&arrayRef)[2]; // expected-warning {{inferring '_Nonnull' for pointer type within array is deprecated}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:14-[[@LINE-1]]:14}:"_Nonnull" + +extern void * (&arrayRef2)[2]; // expected-warning {{inferring '_Nonnull' for pointer type within array is deprecated}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:14-[[@LINE-1]]:14}:" _Nonnull" + +extern CFTypeRef &typedefRef; // expected-warning {{inferring '_Nonnull' for pointer type within reference is deprecated}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:17-[[@LINE-1]]:17}:" _Nonnull" +extern CFTypeRef& typedefRef2; // expected-warning {{inferring '_Nonnull' for pointer type within reference is deprecated}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:17-[[@LINE-1]]:17}:" _Nonnull " + + +void arrayNameless(void *[]); // expected-warning {{inferring '_Nonnull' for pointer type within array is deprecated}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:26-[[@LINE-1]]:26}:"_Nonnull" + +void arrayNameless2(void * []); // expected-warning {{inferring '_Nonnull' for pointer type within array is deprecated}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:27-[[@LINE-1]]:27}:" _Nonnull" + +void arrayNamelessTypedef(CFTypeRef[]); // expected-warning {{inferring '_Nonnull' for pointer type within array is deprecated}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:36-[[@LINE-1]]:36}:" _Nonnull " + +void arrayNamelessTypedef2(CFTypeRef []); // expected-warning {{inferring '_Nonnull' for pointer type within array is deprecated}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:37-[[@LINE-1]]:37}:" _Nonnull" + + +extern int (*pointerToArray)[2]; // no-warning +int checkTypePTA = pointerToArray; // expected-error {{cannot initialize a variable of type 'int' with an lvalue of type 'int (* _Nonnull)[2]'}} + +int **arrayOfNestedPointers[2]; // no-warning +int checkTypeANP = arrayOfNestedPointers; // expected-error {{cannot initialize a variable of type 'int' with an lvalue of type 'int **[2]'}} + +CFTypeRef *arrayOfNestedPointersTypedef[2]; // no-warning +int checkTypeANPT = arrayOfNestedPointersTypedef; // expected-error {{cannot initialize a variable of type 'int' with an lvalue of type 'CFTypeRef *[2]'}} + +#pragma clang assume_nonnull end diff --git a/test/SemaObjCXX/nullability-consistency-arrays.mm b/test/SemaObjCXX/nullability-consistency-arrays.mm index 8ccd191e3b..dc17ce8415 100644 --- a/test/SemaObjCXX/nullability-consistency-arrays.mm +++ b/test/SemaObjCXX/nullability-consistency-arrays.mm @@ -1,9 +1,9 @@ -// RUN: %clang_cc1 -fsyntax-only -fblocks -I %S/Inputs %s -D ARRAYS_CHECKED=1 -verify -// RUN: %clang_cc1 -fsyntax-only -fblocks -I %S/Inputs %s -D ARRAYS_CHECKED=0 -Wno-nullability-completeness-on-arrays -verify -// RUN: %clang_cc1 -fsyntax-only -fblocks -I %S/Inputs %s -Wno-nullability-completeness -Werror -// RUN: %clang_cc1 -fsyntax-only -fblocks -I %S/Inputs -x objective-c %s -D ARRAYS_CHECKED=1 -verify -// RUN: %clang_cc1 -fsyntax-only -fblocks -I %S/Inputs -x objective-c %s -D ARRAYS_CHECKED=0 -Wno-nullability-completeness-on-arrays -verify -// RUN: %clang_cc1 -fsyntax-only -fblocks -I %S/Inputs -x objective-c %s -Wno-nullability-completeness -Werror +// RUN: %clang_cc1 -fsyntax-only -fblocks -I %S/Inputs -Wno-nullability-inferred-on-nested-type %s -D ARRAYS_CHECKED=1 -verify +// RUN: %clang_cc1 -fsyntax-only -fblocks -I %S/Inputs -Wno-nullability-inferred-on-nested-type %s -D ARRAYS_CHECKED=0 -Wno-nullability-completeness-on-arrays -verify +// RUN: %clang_cc1 -fsyntax-only -fblocks -I %S/Inputs -Wno-nullability-inferred-on-nested-type %s -Wno-nullability-completeness -Werror +// RUN: %clang_cc1 -fsyntax-only -fblocks -I %S/Inputs -Wno-nullability-inferred-on-nested-type -x objective-c %s -D ARRAYS_CHECKED=1 -verify +// RUN: %clang_cc1 -fsyntax-only -fblocks -I %S/Inputs -Wno-nullability-inferred-on-nested-type -x objective-c %s -D ARRAYS_CHECKED=0 -Wno-nullability-completeness-on-arrays -verify +// RUN: %clang_cc1 -fsyntax-only -fblocks -I %S/Inputs -Wno-nullability-inferred-on-nested-type -x objective-c %s -Wno-nullability-completeness -Werror #include "nullability-consistency-arrays.h" -- 2.40.0