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">; 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; +def warn_nullability_missing_array : Warning< + "array parameter is missing a nullability type specifier (_Nonnull, " + "_Nullable, or _Null_unspecified)">, + InGroup; def err_objc_type_arg_explicit_nullability : Error< "type argument %0 cannot explicitly specify nullability">; 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,10 +3449,16 @@ // 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(pointerKind); } @@ -3457,8 +3467,43 @@ } // Complain about missing nullability. - S.Diag(pointerLoc, diag::warn_nullability_missing) - << static_cast(pointerKind); + if (pointerKind == SimplePointerKind::Array) { + S.Diag(pointerLoc, diag::warn_nullability_missing_array); + } else { + S.Diag(pointerLoc, diag::warn_nullability_missing) + << static_cast(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(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(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(fileNullability.PointerKind); - } - - fileNullability.SawTypeNullability = true; - } - } - } + if (!implicit) + recordNullabilitySeen(*this, nullabilityLoc); // Check for existing nullability attributes on the type. QualType desugared = type; 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: 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) { }