jordan_rose created this revision. jordan_rose added a reviewer: doug.gregor. jordan_rose added a subscriber: cfe-commits. jordan_rose set the repository for this revision to rL LLVM. jordan_rose added a dependency: D25850: Accept nullability annotations (_Nullable) on array parameters.
This is an addition to (and sub-warning of) -Wnullability-completeness that warns when an array parameter is missing nullability. When the specific warning is switched off, the compiler falls back to only warning on pointer types written as pointer types. Note that use of nullability //within// an array triggers the completeness checks regardless of whether or not the array-specific warning is enabled; the intent there is simply to determine whether a particular header is trying to be nullability-aware at all. Depends on https://reviews.llvm.org/D25850. Part of rdar://problem/25846421. Repository: rL LLVM https://reviews.llvm.org/D26108 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaType.cpp test/SemaObjCXX/Inputs/nullability-consistency-arrays.h test/SemaObjCXX/nullability-consistency-arrays.mm
Index: test/SemaObjCXX/nullability-consistency-arrays.mm =================================================================== --- /dev/null +++ test/SemaObjCXX/nullability-consistency-arrays.mm @@ -0,0 +1,12 @@ +// 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 + +#include "nullability-consistency-arrays.h" + +void h1(int *ptr) { } // don't warn + +void h2(int * _Nonnull p) { } Index: test/SemaObjCXX/Inputs/nullability-consistency-arrays.h =================================================================== --- /dev/null +++ test/SemaObjCXX/Inputs/nullability-consistency-arrays.h @@ -0,0 +1,87 @@ +void firstThingInTheFileThatNeedsNullabilityIsAnArray(int ints[]); +#if ARRAYS_CHECKED +// expected-warning@-2 {{array parameter is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified)}} +#endif + +int *secondThingInTheFileThatNeedsNullabilityIsAPointer; +#if !ARRAYS_CHECKED +// expected-warning@-2 {{pointer is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified)}} +#endif + +int *_Nonnull triggerConsistencyWarnings; + +void test( + int ints[], +#if ARRAYS_CHECKED +// expected-warning@-2 {{array parameter is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified)}} +#endif + void *ptrs[], // expected-warning {{pointer is missing a nullability type specifier}} +#if ARRAYS_CHECKED +// expected-warning@-2 {{array parameter is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified)}} +#endif + void **nestedPtrs[]); // expected-warning 2 {{pointer is missing a nullability type specifier}} +#if ARRAYS_CHECKED +// expected-warning@-2 {{array parameter is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified)}} +#endif + +void testArraysOK( + int ints[_Nonnull], + void *ptrs[_Nonnull], // expected-warning {{pointer is missing a nullability type specifier}} + void **nestedPtrs[_Nonnull]); // expected-warning 2 {{pointer is missing a nullability type specifier}} +void testAllOK( + int ints[_Nonnull], + void * _Nullable ptrs[_Nonnull], + void * _Nullable * _Nullable nestedPtrs[_Nonnull]); + +void nestedArrays(int x[5][1]) {} +#if ARRAYS_CHECKED +// expected-warning@-2 {{array parameter is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified)}} +#endif +void nestedArraysOK(int x[_Nonnull 5][1]) {} + +#if !__cplusplus +void staticOK(int x[static 5][1]){} +#endif + +int globalArraysDoNotNeedNullability[5]; + +typedef int INTS[4]; + +void typedefTest( + INTS x, +#if ARRAYS_CHECKED +// expected-warning@-2 {{array parameter is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified)}} +#endif + INTS _Nonnull x2, + _Nonnull INTS x3, + INTS y[2], +#if ARRAYS_CHECKED +// expected-warning@-2 {{array parameter is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified)}} +#endif + INTS y2[_Nonnull 2]); + + +#pragma clang assume_nonnull begin +void testAssumeNonnull( + int ints[], +#if ARRAYS_CHECKED +// expected-warning@-2 {{array parameter is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified)}} +#endif + void *ptrs[], +#if ARRAYS_CHECKED +// expected-warning@-2 {{array parameter is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified)}} +#endif + void **nestedPtrs[]); // expected-warning 2 {{pointer is missing a nullability type specifier}} +#if ARRAYS_CHECKED +// expected-warning@-2 {{array parameter is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified)}} +#endif + +void testAssumeNonnullAllOK( + int ints[_Nonnull], + void * _Nullable ptrs[_Nonnull], + void * _Nullable * _Nullable nestedPtrs[_Nonnull]); +void testAssumeNonnullAllOK2( + int ints[_Nonnull], + void * ptrs[_Nonnull], // backwards-compatibility + void * _Nullable * _Nullable nestedPtrs[_Nonnull]); +#pragma clang assume_nonnull end Index: lib/Sema/SemaType.cpp =================================================================== --- lib/Sema/SemaType.cpp +++ lib/Sema/SemaType.cpp @@ -3185,6 +3185,7 @@ Pointer, BlockPointer, MemberPointer, + Array, }; } // end anonymous namespace @@ -3430,12 +3431,15 @@ return file; } -/// Check for consistent use of nullability. -static void checkNullabilityConsistency(TypeProcessingState &state, +/// Complains about missing nullability if the file containing \p pointerLoc +/// has other uses of nullability (either the keywords or the \c assume_nonnull +/// pragma). +/// +/// If the file has \e not seen other uses of nullability, this particular +/// pointer is saved for possible later diagnosis. See recordNullabilitySeen(). +static void checkNullabilityConsistency(Sema &S, SimplePointerKind pointerKind, SourceLocation pointerLoc) { - Sema &S = state.getSema(); - // Determine which file we're performing consistency checking for. FileID file = getNullabilityCompletenessCheckFileID(S, pointerLoc); if (file.isInvalid()) @@ -3445,20 +3449,61 @@ // about anything. FileNullability &fileNullability = S.NullabilityMap[file]; if (!fileNullability.SawTypeNullability) { - // If this is the first pointer declarator in the file, record it. + // If this is the first pointer declarator in the file, and the appropriate + // warning is on, record it in case we need to diagnose it retroactively. + diag::kind diagKind; + if (pointerKind == SimplePointerKind::Array) + diagKind = diag::warn_nullability_missing_array; + else + diagKind = diag::warn_nullability_missing; + if (fileNullability.PointerLoc.isInvalid() && - !S.Context.getDiagnostics().isIgnored(diag::warn_nullability_missing, - pointerLoc)) { + !S.Context.getDiagnostics().isIgnored(diagKind, pointerLoc)) { fileNullability.PointerLoc = pointerLoc; fileNullability.PointerKind = static_cast<unsigned>(pointerKind); } return; } // Complain about missing nullability. - S.Diag(pointerLoc, diag::warn_nullability_missing) - << static_cast<unsigned>(pointerKind); + if (pointerKind == SimplePointerKind::Array) { + S.Diag(pointerLoc, diag::warn_nullability_missing_array); + } else { + S.Diag(pointerLoc, diag::warn_nullability_missing) + << static_cast<unsigned>(pointerKind); + } +} + +/// Marks that a nullability feature has been used in the file containing +/// \p loc. +/// +/// If this file already had pointer types in it that were missing nullability, +/// the first such instance is retroactively diagnosed. +/// +/// \sa checkNullabilityConsistency +static void recordNullabilitySeen(Sema &S, SourceLocation loc) { + FileID file = getNullabilityCompletenessCheckFileID(S, loc); + if (file.isInvalid()) + return; + + FileNullability &fileNullability = S.NullabilityMap[file]; + if (fileNullability.SawTypeNullability) + return; + fileNullability.SawTypeNullability = true; + + // If we haven't seen any type nullability before, now we have. Retroactively + // diagnose the first unannotated pointer, if there was one. + if (fileNullability.PointerLoc.isInvalid()) + return; + + if (fileNullability.PointerKind == + static_cast<unsigned>(SimplePointerKind::Array)) { + S.Diag(fileNullability.PointerLoc, diag::warn_nullability_missing_array); + } else { + S.Diag(fileNullability.PointerLoc, diag::warn_nullability_missing) + << fileNullability.PointerKind; + } } /// Returns true if any of the declarator chunks before \p endIndex include a @@ -3571,24 +3616,10 @@ // Are we in an assume-nonnull region? bool inAssumeNonNullRegion = false; - if (S.PP.getPragmaAssumeNonNullLoc().isValid()) { + SourceLocation assumeNonNullLoc = S.PP.getPragmaAssumeNonNullLoc(); + if (assumeNonNullLoc.isValid()) { inAssumeNonNullRegion = true; - // Determine which file we saw the assume-nonnull region in. - FileID file = getNullabilityCompletenessCheckFileID( - S, S.PP.getPragmaAssumeNonNullLoc()); - if (file.isValid()) { - FileNullability &fileNullability = S.NullabilityMap[file]; - - // If we haven't seen any type nullability before, now we have. - if (!fileNullability.SawTypeNullability) { - if (fileNullability.PointerLoc.isValid()) { - S.Diag(fileNullability.PointerLoc, diag::warn_nullability_missing) - << static_cast<unsigned>(fileNullability.PointerKind); - } - - fileNullability.SawTypeNullability = true; - } - } + recordNullabilitySeen(S, assumeNonNullLoc); } // Whether to complain about missing nullability specifiers or not. @@ -3789,27 +3820,35 @@ // Fallthrough. case CAMN_Yes: - checkNullabilityConsistency(state, pointerKind, pointerLoc); + checkNullabilityConsistency(S, pointerKind, pointerLoc); } return nullptr; }; // If the type itself could have nullability but does not, infer pointer // nullability and perform consistency checking. - if (T->canHaveNullability() && S.ActiveTemplateInstantiations.empty() && - !T->getNullability(S.Context)) { - SimplePointerKind pointerKind = SimplePointerKind::Pointer; - if (T->isBlockPointerType()) - pointerKind = SimplePointerKind::BlockPointer; - else if (T->isMemberPointerType()) - pointerKind = SimplePointerKind::MemberPointer; - - if (auto *attr = inferPointerNullability( - pointerKind, D.getDeclSpec().getTypeSpecTypeLoc(), - D.getMutableDeclSpec().getAttributes().getListRef())) { - T = Context.getAttributedType( - AttributedType::getNullabilityAttrKind(*inferNullability), T, T); - attr->setUsedAsTypeAttr(); + if (S.ActiveTemplateInstantiations.empty()) { + if (T->canHaveNullability() && !T->getNullability(S.Context)) { + SimplePointerKind pointerKind = SimplePointerKind::Pointer; + if (T->isBlockPointerType()) + pointerKind = SimplePointerKind::BlockPointer; + else if (T->isMemberPointerType()) + pointerKind = SimplePointerKind::MemberPointer; + + if (auto *attr = inferPointerNullability( + pointerKind, D.getDeclSpec().getTypeSpecTypeLoc(), + D.getMutableDeclSpec().getAttributes().getListRef())) { + T = Context.getAttributedType( + AttributedType::getNullabilityAttrKind(*inferNullability), T, T); + attr->setUsedAsTypeAttr(); + } + } + if (complainAboutMissingNullability == CAMN_Yes && + T->isArrayType() && !T->getNullability(S.Context) && + D.isPrototypeContext() && + !hasOuterPointerLikeChunk(D, D.getNumTypeObjects())) { + checkNullabilityConsistency(S, SimplePointerKind::Array, + D.getDeclSpec().getTypeSpecTypeLoc()); } } @@ -3956,6 +3995,16 @@ break; } + // Array parameters can be marked nullable as well, although it's not + // necessary if they're marked 'static'. + if (complainAboutMissingNullability == CAMN_Yes && + !hasNullabilityAttr(DeclType.getAttrs()) && + ASM != ArrayType::Static && + D.isPrototypeContext() && + !hasOuterPointerLikeChunk(D, chunkIndex)) { + checkNullabilityConsistency(S, SimplePointerKind::Array, DeclType.Loc); + } + T = S.BuildArrayType(T, ASM, ArraySize, ATI.TypeQuals, SourceRange(DeclType.Loc, DeclType.EndLoc), Name); break; @@ -5826,24 +5875,8 @@ bool allowOnArrayType, bool implicit, bool overrideExisting) { - if (!implicit) { - // We saw a nullability type specifier. If this is the first one for - // this file, note that. - FileID file = getNullabilityCompletenessCheckFileID(*this, nullabilityLoc); - if (!file.isInvalid()) { - FileNullability &fileNullability = NullabilityMap[file]; - if (!fileNullability.SawTypeNullability) { - // If we have already seen a pointer declarator without a nullability - // annotation, complain about it. - if (fileNullability.PointerLoc.isValid()) { - Diag(fileNullability.PointerLoc, diag::warn_nullability_missing) - << static_cast<unsigned>(fileNullability.PointerKind); - } - - fileNullability.SawTypeNullability = true; - } - } - } + if (!implicit) + recordNullabilitySeen(*this, nullabilityLoc); // Check for existing nullability attributes on the type. QualType desugared = type; Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -8691,6 +8691,10 @@ "%select{pointer|block pointer|member pointer}0 is missing a nullability " "type specifier (_Nonnull, _Nullable, or _Null_unspecified)">, InGroup<NullabilityCompleteness>; +def warn_nullability_missing_array : Warning< + "array parameter is missing a nullability type specifier (_Nonnull, " + "_Nullable, or _Null_unspecified)">, + InGroup<NullabilityCompletenessOnArrays>; def err_objc_type_arg_explicit_nullability : Error< "type argument %0 cannot explicitly specify nullability">; Index: include/clang/Basic/DiagnosticGroups.td =================================================================== --- include/clang/Basic/DiagnosticGroups.td +++ include/clang/Basic/DiagnosticGroups.td @@ -280,7 +280,9 @@ def Nullability : DiagGroup<"nullability">; def NullabilityDeclSpec : DiagGroup<"nullability-declspec">; def NullableToNonNullConversion : DiagGroup<"nullable-to-nonnull-conversion">; -def NullabilityCompleteness : DiagGroup<"nullability-completeness">; +def NullabilityCompletenessOnArrays : DiagGroup<"nullability-completeness-on-arrays">; +def NullabilityCompleteness : DiagGroup<"nullability-completeness", + [NullabilityCompletenessOnArrays]>; def NullArithmetic : DiagGroup<"null-arithmetic">; def NullCharacter : DiagGroup<"null-character">; def NullDereference : DiagGroup<"null-dereference">;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits