[Sema] Put nullability fix-it after the end of the pointer.

Fixes nullability fix-it for `id<SomeProtocol>`. With this change
nullability specifier is inserted after ">" instead of between
"id" and "<".

rdar://problem/34260995

Reviewers: jordan_rose, doug.gregor, ahatanak, arphaman

Reviewed By: jordan_rose

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D38327

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@314473 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index cd06ae831aaf17896321fb70ed4cc770f03fb384..1b02c61cc6cdb4c950d4d032732d9a9b61b96beb 100644 (file)
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -229,6 +229,10 @@ struct FileNullability {
   /// not have a corresponding nullability annotation.
   SourceLocation PointerLoc;
 
+  /// The end location for the first pointer declarator in the file. Used for
+  /// placing fix-its.
+  SourceLocation PointerEndLoc;
+
   /// Which kind of pointer declarator we saw.
   uint8_t PointerKind;
 

diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp
index d5ec98c8cba6308054e79f7c9fbe7e77d0ac2789..3c455043af39a46a1dd822ee2940bc5fb3ee284c 100644 (file)
--- a/lib/Sema/SemaType.cpp
+++ b/lib/Sema/SemaType.cpp
@@ -3500,7 +3500,8 @@ static void fixItNullability(Sema &S, DiagnosticBuilder &Diag,
 
 static void emitNullabilityConsistencyWarning(Sema &S,
                                               SimplePointerKind PointerKind,
-                                              SourceLocation PointerLoc) {
+                                              SourceLocation PointerLoc,
+                                              SourceLocation PointerEndLoc) {
   assert(PointerLoc.isValid());
 
   if (PointerKind == SimplePointerKind::Array) {
@@ -3510,14 +3511,15 @@ static void emitNullabilityConsistencyWarning(Sema &S,
       << static_cast<unsigned>(PointerKind);
   }
 
-  if (PointerLoc.isMacroID())
+  auto FixItLoc = PointerEndLoc.isValid() ? PointerEndLoc : PointerLoc;
+  if (FixItLoc.isMacroID())
     return;
 
   auto addFixIt = [&](NullabilityKind Nullability) {
-    auto Diag = S.Diag(PointerLoc, diag::note_nullability_fix_it);
+    auto Diag = S.Diag(FixItLoc, diag::note_nullability_fix_it);
     Diag << static_cast<unsigned>(Nullability);
     Diag << static_cast<unsigned>(PointerKind);
-    fixItNullability(S, Diag, PointerLoc, Nullability);
+    fixItNullability(S, Diag, FixItLoc, Nullability);
   };
   addFixIt(NullabilityKind::Nullable);
   addFixIt(NullabilityKind::NonNull);
@@ -3529,9 +3531,10 @@ static void emitNullabilityConsistencyWarning(Sema &S,
 ///
 /// 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) {
+static void
+checkNullabilityConsistency(Sema &S, SimplePointerKind pointerKind,
+                            SourceLocation pointerLoc,
+                            SourceLocation pointerEndLoc = SourceLocation()) {
   // Determine which file we're performing consistency checking for.
   FileID file = getNullabilityCompletenessCheckFileID(S, pointerLoc);
   if (file.isInvalid())
@@ -3552,6 +3555,7 @@ static void checkNullabilityConsistency(Sema &S,
     if (fileNullability.PointerLoc.isInvalid() &&
         !S.Context.getDiagnostics().isIgnored(diagKind, pointerLoc)) {
       fileNullability.PointerLoc = pointerLoc;
+      fileNullability.PointerEndLoc = pointerEndLoc;
       fileNullability.PointerKind = static_cast<unsigned>(pointerKind);
     }
 
@@ -3559,7 +3563,7 @@ static void checkNullabilityConsistency(Sema &S,
   }
 
   // Complain about missing nullability.
-  emitNullabilityConsistencyWarning(S, pointerKind, pointerLoc);
+  emitNullabilityConsistencyWarning(S, pointerKind, pointerLoc, pointerEndLoc);
 }
 
 /// Marks that a nullability feature has been used in the file containing
@@ -3585,7 +3589,8 @@ static void recordNullabilitySeen(Sema &S, SourceLocation loc) {
     return;
 
   auto kind = static_cast<SimplePointerKind>(fileNullability.PointerKind);
-  emitNullabilityConsistencyWarning(S, kind, fileNullability.PointerLoc);
+  emitNullabilityConsistencyWarning(S, kind, fileNullability.PointerLoc,
+                                    fileNullability.PointerEndLoc);
 }
 
 /// Returns true if any of the declarator chunks before \p endIndex include a
@@ -3895,6 +3900,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
   // Returns true if _Nonnull was inferred.
   auto inferPointerNullability = [&](SimplePointerKind pointerKind,
                                      SourceLocation pointerLoc,
+                                     SourceLocation pointerEndLoc,
                                      AttributeList *&attrs) -> AttributeList * {
     // We've seen a pointer.
     if (NumPointersRemaining > 0)
@@ -3950,7 +3956,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
       // Fallthrough.
 
     case CAMN_Yes:
-      checkNullabilityConsistency(S, pointerKind, pointerLoc);
+      checkNullabilityConsistency(S, pointerKind, pointerLoc, pointerEndLoc);
     }
     return nullptr;
   };
@@ -3973,6 +3979,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
 
         if (auto *attr = inferPointerNullability(
               pointerKind, D.getDeclSpec().getTypeSpecTypeLoc(),
+              D.getDeclSpec().getLocEnd(),
               D.getMutableDeclSpec().getAttributes().getListRef())) {
           T = Context.getAttributedType(
                 AttributedType::getNullabilityAttrKind(*inferNullability),T,T);
@@ -4008,8 +4015,8 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
         S.Diag(DeclType.Loc, diag::err_blocks_disable) << LangOpts.OpenCL;
 
       // Handle pointer nullability.
-      inferPointerNullability(SimplePointerKind::BlockPointer,
-                              DeclType.Loc, DeclType.getAttrListRef());
+      inferPointerNullability(SimplePointerKind::BlockPointer, DeclType.Loc,
+                              DeclType.EndLoc, DeclType.getAttrListRef());
 
       T = S.BuildBlockPointerType(T, D.getIdentifierLoc(), Name);
       if (DeclType.Cls.TypeQuals || LangOpts.OpenCL) {
@@ -4031,7 +4038,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
 
       // Handle pointer nullability
       inferPointerNullability(SimplePointerKind::Pointer, DeclType.Loc,
-                              DeclType.getAttrListRef());
+                              DeclType.EndLoc, DeclType.getAttrListRef());
 
       if (LangOpts.ObjC1 && T->getAs<ObjCObjectType>()) {
         T = Context.getObjCObjectPointerType(T);
@@ -4530,8 +4537,8 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
       QualType ClsType;
 
       // Handle pointer nullability.
-      inferPointerNullability(SimplePointerKind::MemberPointer,
-                              DeclType.Loc, DeclType.getAttrListRef());
+      inferPointerNullability(SimplePointerKind::MemberPointer, DeclType.Loc,
+                              DeclType.EndLoc, DeclType.getAttrListRef());
 
       if (SS.isInvalid()) {
         // Avoid emitting extra errors if we already errored on the scope.

diff --git a/test/FixIt/Inputs/nullability-objc.h b/test/FixIt/Inputs/nullability-objc.h
new file mode 100644 (file)
index 0000000..e3e6baa
--- /dev/null
+++ b/test/FixIt/Inputs/nullability-objc.h
@@ -0,0 +1,48 @@
+@class Item;
+@class Container<ObjectType>;
+@protocol Protocol;
+
+// rdar://problem/34260995
+// The first pointer in the file is handled in a different way so need
+// a separate test for this case even if the parameter type is the same as in
+// objcIdParameterWithProtocol.
+void objcIdParameterWithProtocolFirstInFile(id<Protocol> i); // expected-warning {{pointer is missing a nullability type specifier}}
+// expected-note@-1 {{insert '_Nullable'}}
+// expected-note@-2 {{insert '_Nonnull'}}
+// CHECK: fix-it:"{{.*}}nullability-objc.h":{[[@LINE-3]]:57-[[@LINE-3]]:57}:" _Nullable"
+// CHECK: fix-it:"{{.*}}nullability-objc.h":{[[@LINE-4]]:57-[[@LINE-4]]:57}:" _Nonnull"
+
+int * _Nonnull forceNullabilityWarningsObjC(void);
+
+void objcClassParameter(Item *i); // expected-warning {{pointer is missing a nullability type specifier}}
+// expected-note@-1 {{insert '_Nullable'}}
+// expected-note@-2 {{insert '_Nonnull'}}
+// CHECK: fix-it:"{{.*}}nullability-objc.h":{[[@LINE-3]]:31-[[@LINE-3]]:31}:" _Nullable "
+// CHECK: fix-it:"{{.*}}nullability-objc.h":{[[@LINE-4]]:31-[[@LINE-4]]:31}:" _Nonnull "
+
+void objcClassParameterWithProtocol(Item<Protocol> *i); // expected-warning {{pointer is missing a nullability type specifier}}
+// expected-note@-1 {{insert '_Nullable'}}
+// expected-note@-2 {{insert '_Nonnull'}}
+// CHECK: fix-it:"{{.*}}nullability-objc.h":{[[@LINE-3]]:53-[[@LINE-3]]:53}:" _Nullable "
+// CHECK: fix-it:"{{.*}}nullability-objc.h":{[[@LINE-4]]:53-[[@LINE-4]]:53}:" _Nonnull "
+
+// rdar://problem/34260995
+void objcIdParameterWithProtocol(id<Protocol> i); // expected-warning {{pointer is missing a nullability type specifier}}
+// expected-note@-1 {{insert '_Nullable'}}
+// expected-note@-2 {{insert '_Nonnull'}}
+// CHECK: fix-it:"{{.*}}nullability-objc.h":{[[@LINE-3]]:46-[[@LINE-3]]:46}:" _Nullable"
+// CHECK: fix-it:"{{.*}}nullability-objc.h":{[[@LINE-4]]:46-[[@LINE-4]]:46}:" _Nonnull"
+
+// Class parameters don't have nullability type specifier.
+void objcParameterizedClassParameter(Container<Item *> *c); // expected-warning {{pointer is missing a nullability type specifier}}
+// expected-note@-1 {{insert '_Nullable'}}
+// expected-note@-2 {{insert '_Nonnull'}}
+// CHECK: fix-it:"{{.*}}nullability-objc.h":{[[@LINE-3]]:57-[[@LINE-3]]:57}:" _Nullable "
+// CHECK: fix-it:"{{.*}}nullability-objc.h":{[[@LINE-4]]:57-[[@LINE-4]]:57}:" _Nonnull "
+
+// Class parameters don't have nullability type specifier.
+void objcParameterizedClassParameterWithProtocol(Container<id<Protocol>> *c); // expected-warning {{pointer is missing a nullability type specifier}}
+// expected-note@-1 {{insert '_Nullable'}}
+// expected-note@-2 {{insert '_Nonnull'}}
+// CHECK: fix-it:"{{.*}}nullability-objc.h":{[[@LINE-3]]:75-[[@LINE-3]]:75}:" _Nullable "
+// CHECK: fix-it:"{{.*}}nullability-objc.h":{[[@LINE-4]]:75-[[@LINE-4]]:75}:" _Nonnull "

diff --git a/test/FixIt/nullability.mm b/test/FixIt/nullability.mm
index 815c84419243c6289e4cda7a946054c50e66c5ee..a91ccc0f5f640b10f88f02e32f6a8fc710b29b0a 100644 (file)
--- a/test/FixIt/nullability.mm
+++ b/test/FixIt/nullability.mm
@@ -2,8 +2,10 @@
 // RUN: not %clang_cc1 -fdiagnostics-parseable-fixits -fblocks -std=gnu++11 -I %S/Inputs %s >%t.txt 2>&1
 // RUN: FileCheck %s < %t.txt
 // RUN: FileCheck %S/Inputs/nullability.h < %t.txt
+// RUN: FileCheck %S/Inputs/nullability-objc.h < %t.txt
 
 #include "nullability.h"
+#include "nullability-objc.h"
 
 #pragma clang assume_nonnull begin
 

diff --git a/test/SemaObjCXX/Inputs/nullability-consistency-2.h b/test/SemaObjCXX/Inputs/nullability-consistency-2.h
index 72f22e2ce473e4385d1ebc013f6c8ddaea7629ff..84a1369618af0d58cf8d27157b727501225d77df 100644 (file)
--- a/test/SemaObjCXX/Inputs/nullability-consistency-2.h
+++ b/test/SemaObjCXX/Inputs/nullability-consistency-2.h
@@ -6,9 +6,9 @@ void g2(int (^block)(int, int)); // expected-warning{{block pointer is missing a
 
 void g3(const
         id // expected-warning{{missing a nullability type specifier}}
+        volatile
 // expected-note@-1 {{insert '_Nullable' if the pointer may be null}}
 // expected-note@-2 {{insert '_Nonnull' if the pointer should never be null}}
-        volatile
         * // expected-warning{{missing a nullability type specifier}}
 // expected-note@-1 {{insert '_Nullable' if the pointer may be null}}
 // expected-note@-2 {{insert '_Nonnull' if the pointer should never be null}}