[analyzer] Suppress nullability warning for defensive super initializer idiom.
A common idiom in Objective-C initializers is for a defensive nil-check on the
result of a call to a super initializer:
if (self = [super init]) {
...
}
return self;
To avoid warning on this idiom, the nullability checker now suppress diagnostics
for returns of nil on syntactic 'return self' even in initializers with non-null
return types.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@258461 91177308-0d34-0410-b5e6-96231b3b80d8
(cherry picked from commit cd1a0ca84198b972e782f9e3f7827710d9c2573d)
diff --git a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index b532deb..079a71e 100644
--- a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -470,6 +470,22 @@
return E;
}
+/// Returns true when the return statement is a syntactic 'return self' in
+/// Objective-C.
+static bool isReturnSelf(const ReturnStmt *RS, CheckerContext &C) {
+ const ImplicitParamDecl *SelfDecl =
+ C.getCurrentAnalysisDeclContext()->getSelfDecl();
+ if (!SelfDecl)
+ return false;
+
+ const Expr *ReturnExpr = lookThroughImplicitCasts(RS->getRetValue());
+ auto *RefExpr = dyn_cast<DeclRefExpr>(ReturnExpr);
+ if (!RefExpr)
+ return false;
+
+ return RefExpr->getDecl() == SelfDecl;
+}
+
/// This method check when nullable pointer or null value is returned from a
/// function that has nonnull return type.
///
@@ -494,16 +510,28 @@
if (!RetSVal)
return;
+ bool IsReturnSelfInObjCInit = false;
+
QualType RequiredRetType;
AnalysisDeclContext *DeclCtxt =
C.getLocationContext()->getAnalysisDeclContext();
const Decl *D = DeclCtxt->getDecl();
- if (auto *MD = dyn_cast<ObjCMethodDecl>(D))
+ if (auto *MD = dyn_cast<ObjCMethodDecl>(D)) {
RequiredRetType = MD->getReturnType();
- else if (auto *FD = dyn_cast<FunctionDecl>(D))
+ // Suppress diagnostics for returns of nil that are syntactic returns of
+ // self in ObjC initializers. This avoids warning under the common idiom of
+ // a defensive check of the result of a call to super:
+ // if (self = [super init]) {
+ // ...
+ // }
+ // return self; // no-warning
+ IsReturnSelfInObjCInit = (MD->getMethodFamily() == OMF_init) &&
+ isReturnSelf(S, C);
+ } else if (auto *FD = dyn_cast<FunctionDecl>(D)) {
RequiredRetType = FD->getReturnType();
- else
+ } else {
return;
+ }
NullConstraint Nullness = getNullConstraint(*RetSVal, State);
@@ -520,7 +548,8 @@
if (Filter.CheckNullReturnedFromNonnull &&
Nullness == NullConstraint::IsNull &&
RetExprTypeLevelNullability != Nullability::Nonnull &&
- RequiredNullability == Nullability::Nonnull) {
+ RequiredNullability == Nullability::Nonnull &&
+ !IsReturnSelfInObjCInit) {
static CheckerProgramPointTag Tag(this, "NullReturnedFromNonnull");
ExplodedNode *N = C.generateErrorNode(State, &Tag);
if (!N)
diff --git a/test/Analysis/nullability.mm b/test/Analysis/nullability.mm
index aa909fb..b119e63 100644
--- a/test/Analysis/nullability.mm
+++ b/test/Analysis/nullability.mm
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -fobjc-arc -analyze -analyzer-checker=core,nullability -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,nullability -verify %s
#define nil 0
#define BOOL int
@@ -279,10 +280,21 @@
return p;
}
-@interface SomeClass : NSObject
+
+@interface SomeClass : NSObject {
+ int instanceVar;
+}
@end
@implementation SomeClass (MethodReturn)
+- (id)initWithSomething:(int)i {
+ if (self = [super init]) {
+ instanceVar = i;
+ }
+
+ return self;
+}
+
- (TestObject * _Nonnull)testReturnsNullableInNonnullIndirectly {
TestObject *local = getNullableTestObject();
return local; // expected-warning {{Nullable pointer is returned from a function that is expected to return a non-null value}}
@@ -301,3 +313,41 @@
return p; // no-warning
}
@end
+
+@interface ClassWithInitializers : NSObject
+@end
+
+@implementation ClassWithInitializers
+- (instancetype _Nonnull)initWithNonnullReturnAndSelfCheckingIdiom {
+ // This defensive check is a common-enough idiom that we filter don't want
+ // to issue a diagnostic for it,
+ if (self = [super init]) {
+ }
+
+ return self; // no-warning
+}
+
+- (instancetype _Nonnull)initWithNonnullReturnAndNilReturnViaLocal {
+ self = [super init];
+ // This leaks, but we're not checking for that here.
+
+ ClassWithInitializers *other = nil;
+ // Still warn when when not returning via self.
+ return other; // expected-warning {{Null is returned from a function that is expected to return a non-null value}}
+}
+@end
+
+@interface SubClassWithInitializers : ClassWithInitializers
+@end
+
+@implementation SubClassWithInitializers
+// Note: Because this is overridding
+// -[ClassWithInitializers initWithNonnullReturnAndSelfCheckingIdiom],
+// the return type of this method becomes implicitly id _Nonnull.
+- (id)initWithNonnullReturnAndSelfCheckingIdiom {
+ if (self = [super initWithNonnullReturnAndSelfCheckingIdiom]) {
+ }
+
+ return self; // no-warning
+}
+@end
