Skip to content

Commit 2e76562

Browse files
caldacalda
committed
Combine the two diagnostics
1 parent 88b65f4 commit 2e76562

File tree

6 files changed

+60
-73
lines changed

6 files changed

+60
-73
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4007,10 +4007,7 @@ WARNING(use_of_qq_on_non_optional_value,none,
40074007
"left side of nil coalescing operator '?""?' has non-optional type %0, "
40084008
"so the right side is never used", (Type))
40094009
WARNING(nonoptional_compare_to_nil,none,
4010-
"comparing non-optional value of type %0 to 'nil' always returns"
4011-
" %select{false|true}1", (Type, bool))
4012-
WARNING(nonoptional_compare_to_optional_none_case,none,
4013-
"comparing non-optional value of type %0 to 'Optional.none' always returns"
4010+
"comparing non-optional value of type %0 to 'nil' or 'Optional.none' always returns"
40144011
" %select{false|true}1", (Type, bool))
40154012
WARNING(optional_check_nonoptional,none,
40164013
"non-optional expression of type %0 used in a check for optionals",

‎lib/Sema/MiscDiagnostics.cpp

Lines changed: 16 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1282,19 +1282,25 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC,
12821282

12831283
}
12841284

1285-
/// Returns true if this is a 'nil' literal
1286-
bool isNilLiteral(Expr *E) {
1285+
/// Return true if this is a 'nil' literal. This looks
1286+
/// like this if the type is Optional<T>:
1287+
///
1288+
/// (dot_syntax_call_expr implicit type='Int?'
1289+
/// (declref_expr implicit decl=Optional.none)
1290+
/// (type_expr type=Int?))
1291+
///
1292+
/// Or like this if it is any other ExpressibleByNilLiteral type:
1293+
///
1294+
/// (nil_literal_expr)
1295+
///
1296+
bool isTypeCheckedOptionalNil(Expr *E) {
12871297
if (dyn_cast<NilLiteralExpr>(E)) return true;
1288-
return false;
1289-
}
1290-
1291-
/// Returns true if this is a expression is a reference to
1292-
/// the `Optional.none` case specifically (e.g. not `nil`).
1293-
bool isOptionalNoneCase(Expr *E) {
1298+
12941299
auto CE = dyn_cast<ApplyExpr>(E->getSemanticsProvidingExpr());
12951300
if (!CE)
12961301
return false;
12971302

1303+
// First case -- Optional.none
12981304
if (auto DRE = dyn_cast<DeclRefExpr>(CE->getSemanticFn()))
12991305
return DRE->getDecl() == Ctx.getOptionalNoneDecl();
13001306

@@ -1339,12 +1345,9 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC,
13391345

13401346
if (calleeName == "==" || calleeName == "!=" ||
13411347
calleeName == "===" || calleeName == "!==") {
1342-
bool isTrue = calleeName == "!=" || calleeName == "!==";
1343-
1344-
// Diagnose a redundant comparison of a non-optional to a `nil` literal
13451348
if (((subExpr = isImplicitPromotionToOptional(lhs)) &&
1346-
isNilLiteral(rhs)) ||
1347-
(isNilLiteral(lhs) &&
1349+
isTypeCheckedOptionalNil(rhs)) ||
1350+
(isTypeCheckedOptionalNil(lhs) &&
13481351
(subExpr = isImplicitPromotionToOptional(rhs)))) {
13491352
bool isTrue = calleeName == "!=" || calleeName == "!==";
13501353

@@ -1354,19 +1357,6 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC,
13541357
.highlight(rhs->getSourceRange());
13551358
return;
13561359
}
1357-
1358-
// Diagnose a redundant comparison of a non-optional to the `Optional.none` case
1359-
if (((subExpr = isImplicitPromotionToOptional(lhs)) &&
1360-
isOptionalNoneCase(rhs)) ||
1361-
(isOptionalNoneCase(lhs) &&
1362-
(subExpr = isImplicitPromotionToOptional(rhs)))) {
1363-
1364-
Ctx.Diags.diagnose(DRE->getLoc(), diag::nonoptional_compare_to_optional_none_case,
1365-
subExpr->getType(), isTrue)
1366-
.highlight(lhs->getSourceRange())
1367-
.highlight(rhs->getSourceRange());
1368-
return;
1369-
}
13701360
}
13711361
}
13721362
};

‎test/ClangImporter/nullability.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@ import CoreCooling
55
func testSomeClass(_ sc: SomeClass, osc: SomeClass?) {
66
let ao1: Any = sc.methodA(osc)
77
_ = ao1
8-
if sc.methodA(osc) == nil { } // expected-warning {{comparing non-optional value of type 'Any' to 'nil' always returns false}}
9-
if sc.methodA(osc) == Optional.none { } // expected-warning {{comparing non-optional value of type 'Any' to 'Optional.none' always returns false}}
8+
if sc.methodA(osc) == nil { } // expected-warning {{comparing non-optional value of type 'Any' to 'nil' or 'Optional.none' always returns false}}
9+
if sc.methodA(osc) == Optional.none { } // expected-warning {{comparing non-optional value of type 'Any' to 'nil' or 'Optional.none' always returns false}}
1010

1111
let ao2: Any = sc.methodB(nil)
1212
_ = ao2
13-
if sc.methodA(osc) == nil { }// expected-warning {{comparing non-optional value of type 'Any' to 'nil' always returns false}}
13+
if sc.methodA(osc) == nil { }// expected-warning {{comparing non-optional value of type 'Any' to 'nil' or 'Optional.none' always returns false}}
1414

1515
let ao3: Any? = sc.property.flatMap { .some($0) }
1616
_ = ao3
@@ -19,7 +19,7 @@ func testSomeClass(_ sc: SomeClass, osc: SomeClass?) {
1919

2020
let ao4: Any = sc.methodD()
2121
_ = ao4
22-
if sc.methodD() == nil { } // expected-warning {{comparing non-optional value of type 'Any' to 'nil' always returns false}}
22+
if sc.methodD() == nil { } // expected-warning {{comparing non-optional value of type 'Any' to 'nil' or 'Optional.none' always returns false}}
2323

2424
sc.methodE(sc)
2525
sc.methodE(osc) // expected-error{{value of optional type 'SomeClass?' must be unwrapped}}
@@ -44,7 +44,7 @@ func testSomeClass(_ sc: SomeClass, osc: SomeClass?) {
4444
let sc2 = SomeClass(int: ci)
4545
let sc2a: SomeClass = sc2
4646
_ = sc2a
47-
if sc2 == nil { } // expected-warning {{comparing non-optional value of type 'SomeClass' to 'nil' always returns false}}
47+
if sc2 == nil { } // expected-warning {{comparing non-optional value of type 'SomeClass' to 'nil' or 'Optional.none' always returns false}}
4848

4949
let sc3 = SomeClass(double: 1.5)
5050
if sc3 == nil { } // okay
@@ -56,7 +56,7 @@ func testSomeClass(_ sc: SomeClass, osc: SomeClass?) {
5656
let sc4 = sc.returnMe()
5757
let sc4a: SomeClass = sc4
5858
_ = sc4a
59-
if sc4 == nil { } // expected-warning {{comparing non-optional value of type 'SomeClass' to 'nil' always returns false}}
59+
if sc4 == nil { } // expected-warning {{comparing non-optional value of type 'SomeClass' to 'nil' or 'Optional.none' always returns false}}
6060
}
6161

6262
// Nullability with CF types.

‎test/Constraints/diagnostics.swift

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ class r20201968C {
222222
// <rdar://problem/21459429> QoI: Poor compilation error calling assert
223223
func r21459429(_ a : Int) {
224224
assert(a != nil, "ASSERT COMPILATION ERROR")
225-
// expected-warning @-1 {{comparing non-optional value of type 'Int' to 'nil' always returns true}}
225+
// expected-warning @-1 {{comparing non-optional value of type 'Int' to 'nil' or 'Optional.none' always returns true}}
226226
}
227227

228228

@@ -537,7 +537,7 @@ func r21684487() {
537537
func r18397777(_ d : r21447318?) {
538538
let c = r21447318()
539539

540-
if c != nil { // expected-warning {{comparing non-optional value of type 'r21447318' to 'nil' always returns true}}
540+
if c != nil { // expected-warning {{comparing non-optional value of type 'r21447318' to 'nil' or 'Optional.none' always returns true}}
541541
}
542542

543543
if d { // expected-error {{optional type 'r21447318?' cannot be used as a boolean; test for '!= nil' instead}} {{6-6=(}} {{7-7= != nil)}}
@@ -745,27 +745,27 @@ class C_44203 {
745745
func f(bytes : UnsafeMutablePointer<Int>, _ i : Int?) {
746746
_ = (i === nil) // expected-error {{value of type 'Int?' cannot be compared by reference; did you mean to compare by value?}} {{12-15===}}
747747
_ = (bytes === nil) // expected-error {{type 'UnsafeMutablePointer<Int>' is not optional, value can never be nil}}
748-
_ = (self === nil) // expected-warning {{comparing non-optional value of type 'AnyObject' to 'nil' always returns false}}
749-
_ = (self === .none) // expected-warning {{comparing non-optional value of type 'AnyObject' to 'Optional.none' always returns false}}
750-
_ = (self === Optional.none) // expected-warning {{comparing non-optional value of type 'AnyObject' to 'Optional.none' always returns false}}
748+
_ = (self === nil) // expected-warning {{comparing non-optional value of type 'AnyObject' to 'nil' or 'Optional.none' always returns false}}
749+
_ = (self === .none) // expected-warning {{comparing non-optional value of type 'AnyObject' to 'nil' or 'Optional.none' always returns false}}
750+
_ = (self === Optional.none) // expected-warning {{comparing non-optional value of type 'AnyObject' to 'nil' or 'Optional.none' always returns false}}
751751
_ = (i !== nil) // expected-error {{value of type 'Int?' cannot be compared by reference; did you mean to compare by value?}} {{12-15=!=}}
752752
_ = (bytes !== nil) // expected-error {{type 'UnsafeMutablePointer<Int>' is not optional, value can never be nil}}
753-
_ = (self !== nil) // expected-warning {{comparing non-optional value of type 'AnyObject' to 'nil' always returns true}}
754-
_ = (self !== .none) // expected-warning {{comparing non-optional value of type 'AnyObject' to 'Optional.none' always returns true}}
755-
_ = (self !== Optional.none) // expected-warning {{comparing non-optional value of type 'AnyObject' to 'Optional.none' always returns true}}
753+
_ = (self !== nil) // expected-warning {{comparing non-optional value of type 'AnyObject' to 'nil' or 'Optional.none' always returns true}}
754+
_ = (self !== .none) // expected-warning {{comparing non-optional value of type 'AnyObject' to 'nil' or 'Optional.none' always returns true}}
755+
_ = (self !== Optional.none) // expected-warning {{comparing non-optional value of type 'AnyObject' to 'nil' or 'Optional.none' always returns true}}
756756
}
757757
}
758758

759759
func nilComparison(i: Int, o: AnyObject) {
760-
_ = i == nil // expected-warning {{comparing non-optional value of type 'Int' to 'nil' always returns false}}
761-
_ = nil == i // expected-warning {{comparing non-optional value of type 'Int' to 'nil' always returns false}}
762-
_ = i != nil // expected-warning {{comparing non-optional value of type 'Int' to 'nil' always returns true}}
763-
_ = nil != i // expected-warning {{comparing non-optional value of type 'Int' to 'nil' always returns true}}
764-
765-
_ = i == Optional.none // expected-warning {{comparing non-optional value of type 'Int' to 'Optional.none' always returns false}}
766-
_ = Optional.none == i // expected-warning {{comparing non-optional value of type 'Int' to 'Optional.none' always returns false}}
767-
_ = i != Optional.none // expected-warning {{comparing non-optional value of type 'Int' to 'Optional.none' always returns true}}
768-
_ = Optional.none != i // expected-warning {{comparing non-optional value of type 'Int' to 'Optional.none' always returns true}}
760+
_ = i == nil // expected-warning {{comparing non-optional value of type 'Int' to 'nil' or 'Optional.none' always returns false}}
761+
_ = nil == i // expected-warning {{comparing non-optional value of type 'Int' to 'nil' or 'Optional.none' always returns false}}
762+
_ = i != nil // expected-warning {{comparing non-optional value of type 'Int' to 'nil' or 'Optional.none' always returns true}}
763+
_ = nil != i // expected-warning {{comparing non-optional value of type 'Int' to 'nil' or 'Optional.none' always returns true}}
764+
765+
_ = i == Optional.none // expected-warning {{comparing non-optional value of type 'Int' to 'nil' or 'Optional.none' always returns false}}
766+
_ = Optional.none == i // expected-warning {{comparing non-optional value of type 'Int' to 'nil' or 'Optional.none' always returns false}}
767+
_ = i != Optional.none // expected-warning {{comparing non-optional value of type 'Int' to 'nil' or 'Optional.none' always returns true}}
768+
_ = Optional.none != i // expected-warning {{comparing non-optional value of type 'Int' to 'nil' or 'Optional.none' always returns true}}
769769

770770
// FIXME(integers): uncomment these tests once the < is no longer ambiguous
771771
// _ = i < nil // _xpected-error {{type 'Int' is not optional, value can never be nil}}
@@ -777,8 +777,8 @@ func nilComparison(i: Int, o: AnyObject) {
777777
// _ = i >= nil // _xpected-error {{type 'Int' is not optional, value can never be nil}}
778778
// _ = nil >= i // _xpected-error {{type 'Int' is not optional, value can never be nil}}
779779

780-
_ = o === nil // expected-warning {{comparing non-optional value of type 'AnyObject' to 'nil' always returns false}}
781-
_ = o !== nil // expected-warning {{comparing non-optional value of type 'AnyObject' to 'nil' always returns true}}
780+
_ = o === nil // expected-warning {{comparing non-optional value of type 'AnyObject' to 'nil' or 'Optional.none' always returns false}}
781+
_ = o !== nil // expected-warning {{comparing non-optional value of type 'AnyObject' to 'nil' or 'Optional.none' always returns true}}
782782
}
783783

784784
// <rdar://problem/23709100> QoI: incorrect ambiguity error due to implicit conversion

‎test/expr/cast/nil_value_to_optional.swift

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ var f = false
55

66
func markUsed<T>(_ t: T) {}
77

8-
markUsed(t != nil) // expected-warning {{comparing non-optional value of type 'Bool' to 'nil' always returns true}}
9-
markUsed(f != nil) // expected-warning {{comparing non-optional value of type 'Bool' to 'nil' always returns true}}
10-
markUsed(t != Optional.none) // expected-warning {{comparing non-optional value of type 'Bool' to 'Optional.none' always returns true}}
11-
markUsed(f != Optional.none) // expected-warning {{comparing non-optional value of type 'Bool' to 'Optional.none' always returns true}}
8+
markUsed(t != nil) // expected-warning {{comparing non-optional value of type 'Bool' to 'nil' or 'Optional.none' always returns true}}
9+
markUsed(f != nil) // expected-warning {{comparing non-optional value of type 'Bool' to 'nil' or 'Optional.none' always returns true}}
10+
markUsed(t != Optional.none) // expected-warning {{comparing non-optional value of type 'Bool' to 'nil' or 'Optional.none' always returns true}}
11+
markUsed(f != Optional.none) // expected-warning {{comparing non-optional value of type 'Bool' to 'nil' or 'Optional.none' always returns true}}
1212

1313
class C : Equatable {}
1414

@@ -17,10 +17,10 @@ func == (lhs: C, rhs: C) -> Bool {
1717
}
1818

1919
func test(_ c: C) {
20-
if c == nil {} // expected-warning {{comparing non-optional value of type 'C' to 'nil' always returns false}}
21-
if c == .none {} // expected-warning {{comparing non-optional value of type 'C' to 'Optional.none' always returns false}}
22-
if c == Optional.none {} // expected-warning {{comparing non-optional value of type 'C' to 'Optional.none' always returns false}}
23-
if c == C?.none {} // expected-warning {{comparing non-optional value of type 'C' to 'Optional.none' always returns false}}
20+
if c == nil {} // expected-warning {{comparing non-optional value of type 'C' to 'nil' or 'Optional.none' always returns false}}
21+
if c == .none {} // expected-warning {{comparing non-optional value of type 'C' to 'nil' or 'Optional.none' always returns false}}
22+
if c == Optional.none {} // expected-warning {{comparing non-optional value of type 'C' to 'nil' or 'Optional.none' always returns false}}
23+
if c == C?.none {} // expected-warning {{comparing non-optional value of type 'C' to 'nil' or 'Optional.none' always returns false}}
2424
}
2525

2626
class D {}
@@ -34,4 +34,4 @@ _ = d! // expected-error {{cannot force unwrap value of non-optional type 'D'}}
3434
_ = dopt == nil
3535
_ = diuopt == nil
3636
_ = diuopt is ExpressibleByNilLiteral // expected-warning {{'is' test is always true}}
37-
_ = produceD() is ExpressibleByNilLiteral // expected-warning {{'is' test is always true}}
37+
_ = produceD() is ExpressibleByNilLiteral // expected-warning {{'is' test is always true}}

‎test/expr/expressions.swift

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -762,15 +762,15 @@ func invalidDictionaryLiteral() {
762762
//===----------------------------------------------------------------------===//
763763
// nil/metatype comparisons
764764
//===----------------------------------------------------------------------===//
765-
_ = Int.self == nil // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' always returns false}}
766-
_ = nil == Int.self // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' always returns false}}
767-
_ = Int.self != nil // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' always returns true}}
768-
_ = nil != Int.self // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' always returns true}}
769-
770-
_ = Int.self == .none // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'Optional.none' always returns false}}
771-
_ = .none == Int.self // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'Optional.none' always returns false}}
772-
_ = Int.self != .none // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'Optional.none' always returns true}}
773-
_ = .none != Int.self // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'Optional.none' always returns true}}
765+
_ = Int.self == nil // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' or 'Optional.none' always returns false}}
766+
_ = nil == Int.self // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' or 'Optional.none' always returns false}}
767+
_ = Int.self != nil // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' or 'Optional.none' always returns true}}
768+
_ = nil != Int.self // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' or 'Optional.none' always returns true}}
769+
770+
_ = Int.self == .none // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' or 'Optional.none' always returns false}}
771+
_ = .none == Int.self // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' or 'Optional.none' always returns false}}
772+
_ = Int.self != .none // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' or 'Optional.none' always returns true}}
773+
_ = .none != Int.self // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' or 'Optional.none' always returns true}}
774774

775775
// <rdar://problem/19032294> Disallow postfix ? when not chaining
776776
func testOptionalChaining(_ a : Int?, b : Int!, c : Int??) {

0 commit comments

Comments
 (0)