r314473 - [Sema] Put nullability fix-it after the end of the pointer.
Volodymyr Sapsai via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 28 16:18:49 PDT 2017
Author: vsapsai
Date: Thu Sep 28 16:18:49 2017
New Revision: 314473
URL: http://llvm.org/viewvc/llvm-project?rev=314473&view=rev
Log:
[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
Added:
cfe/trunk/test/FixIt/Inputs/nullability-objc.h
Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/FixIt/nullability.mm
cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-2.h
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=314473&r1=314472&r2=314473&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Sep 28 16:18:49 2017
@@ -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;
Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=314473&r1=314472&r2=314473&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Thu Sep 28 16:18:49 2017
@@ -3500,7 +3500,8 @@ static void fixItNullability(Sema &S, Di
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 emitNullabilityConsistencyWa
<< 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 emitNullabilityConsistencyWa
///
/// 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(
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(
}
// 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 &
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 *GetFullTypeForDec
// 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 *GetFullTypeForDec
// Fallthrough.
case CAMN_Yes:
- checkNullabilityConsistency(S, pointerKind, pointerLoc);
+ checkNullabilityConsistency(S, pointerKind, pointerLoc, pointerEndLoc);
}
return nullptr;
};
@@ -3973,6 +3979,7 @@ static TypeSourceInfo *GetFullTypeForDec
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 *GetFullTypeForDec
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 *GetFullTypeForDec
// 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 *GetFullTypeForDec
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.
Added: cfe/trunk/test/FixIt/Inputs/nullability-objc.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/Inputs/nullability-objc.h?rev=314473&view=auto
==============================================================================
--- cfe/trunk/test/FixIt/Inputs/nullability-objc.h (added)
+++ cfe/trunk/test/FixIt/Inputs/nullability-objc.h Thu Sep 28 16:18:49 2017
@@ -0,0 +1,48 @@
+ at class Item;
+ at class Container<ObjectType>;
+ at 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 at -1 {{insert '_Nullable'}}
+// expected-note at -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 at -1 {{insert '_Nullable'}}
+// expected-note at -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 at -1 {{insert '_Nullable'}}
+// expected-note at -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 at -1 {{insert '_Nullable'}}
+// expected-note at -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 at -1 {{insert '_Nullable'}}
+// expected-note at -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 at -1 {{insert '_Nullable'}}
+// expected-note at -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 "
Modified: cfe/trunk/test/FixIt/nullability.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/nullability.mm?rev=314473&r1=314472&r2=314473&view=diff
==============================================================================
--- cfe/trunk/test/FixIt/nullability.mm (original)
+++ cfe/trunk/test/FixIt/nullability.mm Thu Sep 28 16:18:49 2017
@@ -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
Modified: cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-2.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-2.h?rev=314473&r1=314472&r2=314473&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-2.h (original)
+++ cfe/trunk/test/SemaObjCXX/Inputs/nullability-consistency-2.h Thu Sep 28 16:18:49 2017
@@ -6,9 +6,9 @@ void g2(int (^block)(int, int)); // expe
void g3(const
id // expected-warning{{missing a nullability type specifier}}
+ volatile
// expected-note at -1 {{insert '_Nullable' if the pointer may be null}}
// expected-note at -2 {{insert '_Nonnull' if the pointer should never be null}}
- volatile
* // expected-warning{{missing a nullability type specifier}}
// expected-note at -1 {{insert '_Nullable' if the pointer may be null}}
// expected-note at -2 {{insert '_Nonnull' if the pointer should never be null}}
More information about the cfe-commits
mailing list