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

llvm-svn: 290141
This commit is contained in:
Jordan Rose 2016-12-19 22:35:24 +00:00
parent 5b1ee2fad9
commit 281139c159
3 changed files with 48 additions and 22 deletions

View File

@ -3443,31 +3443,39 @@ static FileID getNullabilityCompletenessCheckFileID(Sema &S,
/// 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 emitNullabilityConsistencyWarning(Sema &S,
<< 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 *GetFullTypeForDeclarator(TypeProcessingState &state,
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)

View File

@ -17,3 +17,11 @@ void arrayParameterWithStar(int x[*]); // expected-warning {{array parameter is
// expected-note@-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

View File

@ -16,6 +16,12 @@ extern void* array2[2]; // expected-warning {{inferring '_Nonnull' for pointer t
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;