r286520 - Add -Wnullability-completeness-on-arrays.

Jordan Rose via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 10 15:28:26 PST 2016


Author: jrose
Date: Thu Nov 10 17:28:26 2016
New Revision: 286520

URL: http://llvm.org/viewvc/llvm-project?rev=286520&view=rev
Log:
Add -Wnullability-completeness-on-arrays.

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.

Part of rdar://problem/25846421.

Added:
    cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-arrays.h
    cfe/trunk/test/SemaObjCXX/nullability-consistency-arrays.mm
Modified:
    cfe/trunk/include/clang/Basic/DiagnosticGroups.td
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Sema/SemaType.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=286520&r1=286519&r2=286520&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Thu Nov 10 17:28:26 2016
@@ -278,7 +278,9 @@ def NewlineEOF : DiagGroup<"newline-eof"
 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">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=286520&r1=286519&r2=286520&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Nov 10 17:28:26 2016
@@ -8732,6 +8732,10 @@ def warn_nullability_missing : Warning<
   "%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">;

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=286520&r1=286519&r2=286520&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Thu Nov 10 17:28:26 2016
@@ -3212,6 +3212,7 @@ namespace {
     Pointer,
     BlockPointer,
     MemberPointer,
+    Array,
   };
 } // end anonymous namespace
 
@@ -3453,12 +3454,15 @@ static FileID getNullabilityCompleteness
   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())
@@ -3468,10 +3472,16 @@ static void checkNullabilityConsistency(
   // 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);
     }
@@ -3480,8 +3490,43 @@ static void checkNullabilityConsistency(
   }
 
   // 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
@@ -3594,24 +3639,10 @@ static TypeSourceInfo *GetFullTypeForDec
 
   // 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.
@@ -3822,27 +3853,35 @@ static TypeSourceInfo *GetFullTypeForDec
       // 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());
     }
   }
 
@@ -3989,6 +4028,16 @@ static TypeSourceInfo *GetFullTypeForDec
         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;
@@ -5851,22 +5900,7 @@ bool Sema::checkNullabilityTypeSpecifier
                                          SourceLocation nullabilityLoc,
                                          bool isContextSensitive,
                                          bool allowOnArrayType) {
-  // 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;
-    }
-  }
+  recordNullabilitySeen(*this, nullabilityLoc);
 
   // Check for existing nullability attributes on the type.
   QualType desugared = type;

Added: cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-arrays.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-arrays.h?rev=286520&view=auto
==============================================================================
--- cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-arrays.h (added)
+++ cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-arrays.h Thu Nov 10 17:28:26 2016
@@ -0,0 +1,87 @@
+void firstThingInTheFileThatNeedsNullabilityIsAnArray(int ints[]);
+#if ARRAYS_CHECKED
+// expected-warning at -2 {{array parameter is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified)}}
+#endif
+
+int *secondThingInTheFileThatNeedsNullabilityIsAPointer;
+#if !ARRAYS_CHECKED
+// expected-warning at -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 at -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 at -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 at -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 at -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 at -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 at -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 at -2 {{array parameter is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified)}}
+#endif
+  void *ptrs[],
+#if ARRAYS_CHECKED
+// expected-warning at -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 at -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

Added: cfe/trunk/test/SemaObjCXX/nullability-consistency-arrays.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/nullability-consistency-arrays.mm?rev=286520&view=auto
==============================================================================
--- cfe/trunk/test/SemaObjCXX/nullability-consistency-arrays.mm (added)
+++ cfe/trunk/test/SemaObjCXX/nullability-consistency-arrays.mm Thu Nov 10 17:28:26 2016
@@ -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) { }




More information about the cfe-commits mailing list