r290141 - Don't try to emit nullability fix-its within/around macros.

[Jordan Rose via cfe-commits]

Author: jrose
Date: Mon Dec 19 16:35:24 2016
New Revision: 290141

URL: http://llvm.org/viewvc/llvm-project?rev=290141&view=rev
Log:
Don't try to emit nullability fix-its within/around macros.

The newly-added notes from r290132 are too noisy even when the fix-it
is valid. For the existing warning from r286521, it's probably the
right decision 95% of the time to put the change outside the macro if
the array is outside the macro and inside otherwise, but I don't want
to overthink it right now.

Caught by the ASan bot!

More rdar://problem/29524992

Modified:
    cfe/trunk/lib/Sema/SemaType.cpp
    cfe/trunk/test/FixIt/Inputs/nullability.h
    cfe/trunk/test/FixIt/nullability.mm

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=290141&r1=290140&r2=290141&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Mon Dec 19 16:35:24 2016
@@ -3443,31 +3443,39 @@ static FileID getNullabilityCompleteness
 
 /// Creates a fix-it to insert a C-style nullability keyword at \p pointerLoc,
 /// taking into account whitespace before and after.
-static FixItHint fixItNullability(Sema &S, SourceLocation PointerLoc,
-                                  NullabilityKind Nullability) {
+static void fixItNullability(Sema &S, DiagnosticBuilder &Diag,
+                             SourceLocation PointerLoc,
+                             NullabilityKind Nullability) {
   assert(PointerLoc.isValid());
+  if (PointerLoc.isMacroID())
+    return;
+
+  SourceLocation FixItLoc = S.getLocForEndOfToken(PointerLoc);
+  if (!FixItLoc.isValid() || FixItLoc == PointerLoc)
+    return;
+
+  const char *NextChar = S.SourceMgr.getCharacterData(FixItLoc);
+  if (!NextChar)
+    return;
 
   SmallString<32> InsertionTextBuf{" "};
   InsertionTextBuf += getNullabilitySpelling(Nullability);
   InsertionTextBuf += " ";
   StringRef InsertionText = InsertionTextBuf.str();
 
-  SourceLocation FixItLoc = S.getLocForEndOfToken(PointerLoc);
-  if (const char *NextChar = S.SourceMgr.getCharacterData(FixItLoc)) {
-    if (isWhitespace(*NextChar)) {
-      InsertionText = InsertionText.drop_back();
-    } else if (NextChar[-1] == '[') {
-      if (NextChar[0] == ']')
-        InsertionText = InsertionText.drop_back().drop_front();
-      else
-        InsertionText = InsertionText.drop_front();
-    } else if (!isIdentifierBody(NextChar[0], /*allow dollar*/true) &&
-               !isIdentifierBody(NextChar[-1], /*allow dollar*/true)) {
+  if (isWhitespace(*NextChar)) {
+    InsertionText = InsertionText.drop_back();
+  } else if (NextChar[-1] == '[') {
+    if (NextChar[0] == ']')
       InsertionText = InsertionText.drop_back().drop_front();
-    }
+    else
+      InsertionText = InsertionText.drop_front();
+  } else if (!isIdentifierBody(NextChar[0], /*allow dollar*/true) &&
+             !isIdentifierBody(NextChar[-1], /*allow dollar*/true)) {
+    InsertionText = InsertionText.drop_back().drop_front();
   }
 
-  return FixItHint::CreateInsertion(FixItLoc, InsertionText);
+  Diag << FixItHint::CreateInsertion(FixItLoc, InsertionText);
 }
 
 static void emitNullabilityConsistencyWarning(Sema &S,
@@ -3482,11 +3490,14 @@ static void emitNullabilityConsistencyWa
       << static_cast<unsigned>(PointerKind);
   }
 
+  if (PointerLoc.isMacroID())
+    return;
+
   auto addFixIt = [&](NullabilityKind Nullability) {
-    S.Diag(PointerLoc, diag::note_nullability_fix_it)
-      << static_cast<unsigned>(Nullability)
-      << static_cast<unsigned>(PointerKind)
-      << fixItNullability(S, PointerLoc, Nullability);
+    auto Diag = S.Diag(PointerLoc, diag::note_nullability_fix_it);
+    Diag << static_cast<unsigned>(Nullability);
+    Diag << static_cast<unsigned>(PointerKind);
+    fixItNullability(S, Diag, PointerLoc, Nullability);
   };
   addFixIt(NullabilityKind::Nullable);
   addFixIt(NullabilityKind::NonNull);
@@ -3888,9 +3899,10 @@ static TypeSourceInfo *GetFullTypeForDec
       if (pointerLoc.isValid() &&
           complainAboutInferringWithinChunk !=
             PointerWrappingDeclaratorKind::None) {
-        S.Diag(pointerLoc, diag::warn_nullability_inferred_on_nested_type)
-          << static_cast<int>(complainAboutInferringWithinChunk)
-          << fixItNullability(S, pointerLoc, NullabilityKind::NonNull);
+        auto Diag =
+            S.Diag(pointerLoc, diag::warn_nullability_inferred_on_nested_type);
+        Diag << static_cast<int>(complainAboutInferringWithinChunk);
+        fixItNullability(S, Diag, pointerLoc, NullabilityKind::NonNull);
       }
 
       if (inferNullabilityInnerOnly)

Modified: cfe/trunk/test/FixIt/Inputs/nullability.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/Inputs/nullability.h?rev=290141&r1=290140&r2=290141&view=diff
==============================================================================
--- cfe/trunk/test/FixIt/Inputs/nullability.h (original)
+++ cfe/trunk/test/FixIt/Inputs/nullability.h Mon Dec 19 16:35:24 2016
@@ -17,3 +17,11 @@ void arrayParameterWithStar(int x[*]); /
 // expected-note at -2 {{insert '_Nonnull'}}
 // CHECK: fix-it:"{{.*}}nullability.h":{[[@LINE-3]]:35-[[@LINE-3]]:35}:"_Nullable "
 // CHECK: fix-it:"{{.*}}nullability.h":{[[@LINE-4]]:35-[[@LINE-4]]:35}:"_Nonnull "
+
+
+// No fix-its on either the macro definition or instantiation.
+// CHECK-NOT: fix-it:"{{.*}}nullability.h":{[[@LINE+2]]
+// CHECK-NOT: fix-it:"{{.*}}nullability.h":{[[@LINE+2]]
+#define PTR(X) X *
+PTR(int) a; // expected-warning{{pointer is missing a nullability type specifier}}
+#undef PTR

Modified: cfe/trunk/test/FixIt/nullability.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/nullability.mm?rev=290141&r1=290140&r2=290141&view=diff
==============================================================================
--- cfe/trunk/test/FixIt/nullability.mm (original)
+++ cfe/trunk/test/FixIt/nullability.mm Mon Dec 19 16:35:24 2016
@@ -16,6 +16,12 @@ extern void* array2[2]; // expected-warn
 extern void *nestedArray[2][3]; // expected-warning {{inferring '_Nonnull' for pointer type within array is deprecated}}
 // CHECK: fix-it:"{{.*}}nullability.mm":{[[@LINE-1]]:14-[[@LINE-1]]:14}:" _Nonnull "
 
+// No fix-its on either the macro definition or instantiation.
+// CHECK-NOT: fix-it:"{{.*}}nullability.mm":{[[@LINE+2]]
+// CHECK-NOT: fix-it:"{{.*}}nullability.mm":{[[@LINE+2]]
+#define PTR(X) X *
+extern PTR(void) array[2]; // expected-warning {{inferring '_Nonnull' for pointer type within array is deprecated}}
+
 
 typedef const void *CFTypeRef;