Skip to content

[SR-1594] Provide a better diagnostic for nil comparisons. #2755

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 1, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,12 @@ ERROR(expression_too_complex,none,
"expression was too complex to be solved in reasonable time; "
"consider breaking up the expression into distinct sub-expressions", ())

ERROR(comparison_with_nil_illegal,none,
"value of type %0 can never be nil, comparison isn't allowed",
ERROR(value_type_comparison_with_nil_illegal_did_you_mean,none,
"value of type %0 cannot be compared by reference; "
"did you mean to compare by value?",
(Type))
ERROR(value_type_comparison_with_nil_illegal,none,
"type %0 is not optional, value can never be nil",
(Type))

ERROR(cannot_match_expr_pattern_with_value,none,
Expand Down
21 changes: 19 additions & 2 deletions lib/Sema/CSDiag.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4311,8 +4311,25 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
if (isa<NilLiteralExpr>(rhsExpr->getValueProvidingExpr()) &&
!isUnresolvedOrTypeVarType(lhsType)) {
if (isNameOfStandardComparisonOperator(overloadName)) {
diagnose(callExpr->getLoc(), diag::comparison_with_nil_illegal, lhsType)
.highlight(lhsExpr->getSourceRange());
// Regardless of whether the type has reference or value semantics,
// comparison with nil is illegal, albeit for different reasons spelled
// out by the diagnosis.
if (lhsType->getAnyOptionalObjectType() &&
(overloadName == "!==" || overloadName == "===")) {
auto revisedName = overloadName;
revisedName.pop_back();
// If we made it here, then we're trying to perform a comparison with
// reference semantics rather than value semantics. The fixit will
// lop off the extra '=' in the operator.
diagnose(callExpr->getLoc(),
diag::value_type_comparison_with_nil_illegal_did_you_mean,
lhsType)
.fixItReplace(callExpr->getLoc(), revisedName);
} else {
diagnose(callExpr->getLoc(),
diag::value_type_comparison_with_nil_illegal, lhsType)
.highlight(lhsExpr->getSourceRange());
}
return true;
}
}
Expand Down
10 changes: 5 additions & 5 deletions test/ClangModules/nullability.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@ import CoreCooling

func testSomeClass(_ sc: SomeClass, osc: SomeClass?) {
var ao1: AnyObject = sc.methodA(osc)
if sc.methodA(osc) == nil { } // expected-error{{value of type 'AnyObject' can never be nil, comparison isn't allowed}}
if sc.methodA(osc) == nil { } // expected-error{{type 'AnyObject' is not optional, value can never be nil}}

var ao2: AnyObject = sc.methodB(nil)
if sc.methodA(osc) == nil { } // expected-error{{value of type 'AnyObject' can never be nil, comparison isn't allowed}}
if sc.methodA(osc) == nil { } // expected-error{{type 'AnyObject' is not optional, value can never be nil}}

var ao3: AnyObject = sc.property // expected-error{{value of optional type 'AnyObject?' not unwrapped; did you mean to use '!' or '?'?}} {{35-35=!}}
var ao3_ok: AnyObject? = sc.property // okay

var ao4: AnyObject = sc.methodD()
if sc.methodD() == nil { } // expected-error{{value of type 'AnyObject' can never be nil, comparison isn't allowed}}
if sc.methodD() == nil { } // expected-error{{type 'AnyObject' is not optional, value can never be nil}}

sc.methodE(sc)
sc.methodE(osc) // expected-error{{value of optional type 'SomeClass?' not unwrapped; did you mean to use '!' or '?'?}} {{17-17=!}}
Expand All @@ -31,15 +31,15 @@ func testSomeClass(_ sc: SomeClass, osc: SomeClass?) {
let ci: CInt = 1
var sc2 = SomeClass(int: ci)
var sc2a: SomeClass = sc2
if sc2 == nil { } // expected-error{{value of type 'SomeClass' can never be nil, comparison isn't allowed}}
if sc2 == nil { } // expected-error{{type 'SomeClass' is not optional, value can never be nil}}

var sc3 = SomeClass(double: 1.5)
if sc3 == nil { } // okay
var sc3a: SomeClass = sc3 // expected-error{{value of optional type 'SomeClass?' not unwrapped}} {{28-28=!}}

var sc4 = sc.returnMe()
var sc4a: SomeClass = sc4
if sc4 == nil { } // expected-error{{value of type 'SomeClass' can never be nil, comparison isn't allowed}}
if sc4 == nil { } // expected-error{{type 'SomeClass' is not optional, value can never be nil}}
}

// Nullability with CF types.
Expand Down
2 changes: 1 addition & 1 deletion test/ClangModules/objc_failable_inits.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import Foundation
func testDictionary() {
// -[NSDictionary init] returns non-nil.
var dictNonOpt = NSDictionary()
if dictNonOpt == nil { } // expected-error{{value of type 'NSDictionary' can never be nil, comparison isn't allowed}}
if dictNonOpt == nil { } // expected-error{{type 'NSDictionary' is not optional, value can never be nil}}
}

func testString() throws {
Expand Down
17 changes: 15 additions & 2 deletions test/Constraints/diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ class r20201968C {

// <rdar://problem/21459429> QoI: Poor compilation error calling assert
func r21459429(_ a : Int) {
assert(a != nil, "ASSERT COMPILATION ERROR") // expected-error {{value of type 'Int' can never be nil, comparison isn't allowed}}
assert(a != nil, "ASSERT COMPILATION ERROR") // expected-error {{type 'Int' is not optional, value can never be nil}}
}


Expand Down Expand Up @@ -590,7 +590,7 @@ func r21684487() {
func r18397777(_ d : r21447318?) {
let c = r21447318()

if c != nil { // expected-error {{value of type 'r21447318' can never be nil, comparison isn't allowed}}
if c != nil { // expected-error {{type 'r21447318' is not optional, value can never be nil}}
}

if d { // expected-error {{optional type 'r21447318?' cannot be used as a boolean; test for '!= nil' instead}} {{6-6=(}} {{7-7= != nil)}}
Expand Down Expand Up @@ -765,3 +765,16 @@ func r21523291(_ bytes : UnsafeMutablePointer<UInt8>) {
}


// SR-1594: Wrong error description when using === on non-class types
class SR1594 {
func sr1594(bytes : UnsafeMutablePointer<Int>, _ i : Int?) {
_ = (i === nil) // expected-error {{value of type 'Int?' cannot be compared by reference; did you mean to compare by value?}} {{12-15===}}
_ = (bytes === nil) // expected-error {{type 'UnsafeMutablePointer<Int>' is not optional, value can never be nil}}
_ = (self === nil) // expected-error {{type 'SR1594' is not optional, value can never be nil}}
_ = (i !== nil) // expected-error {{value of type 'Int?' cannot be compared by reference; did you mean to compare by value?}} {{12-15=!=}}
_ = (bytes !== nil) // expected-error {{type 'UnsafeMutablePointer<Int>' is not optional, value can never be nil}}
_ = (self !== nil) // expected-error {{type 'SR1594' is not optional, value can never be nil}}
}
}


8 changes: 4 additions & 4 deletions test/expr/cast/nil_value_to_optional.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ var f = false

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

markUsed(t != nil) // expected-error {{value of type 'Bool' can never be nil, comparison isn't allowed}}
markUsed(f != nil) // expected-error {{value of type 'Bool' can never be nil, comparison isn't allowed}}
markUsed(t != nil) // expected-error {{type 'Bool' is not optional, value can never be nil}}
markUsed(f != nil) // expected-error {{type 'Bool' is not optional, value can never be nil}}

class C : Equatable {}

Expand All @@ -15,7 +15,7 @@ func == (lhs: C, rhs: C) -> Bool {
}

func test(_ c: C) {
if c == nil {} // expected-error {{value of type 'C' can never be nil, comparison isn't allowed}}
if c == nil {} // expected-error {{type 'C' is not optional, value can never be nil}}
}

class D {}
Expand All @@ -24,6 +24,6 @@ var d = D()
var dopt: D? = nil
var diuopt: D! = nil

_ = d == nil // expected-error{{value of type 'D' can never be nil, comparison isn't allowed}}
_ = d == nil // expected-error{{type 'D' is not optional, value can never be nil}}
_ = dopt == nil
_ = diuopt == nil
4 changes: 2 additions & 2 deletions test/expr/expressions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -682,10 +682,10 @@ func invalidDictionaryLiteral() {
//===----------------------------------------------------------------------===//
// nil/metatype comparisons
//===----------------------------------------------------------------------===//
Int.self == nil // expected-error {{value of type 'Int.Type' can never be nil, comparison isn't allowed}}
Int.self == nil // expected-error {{type 'Int.Type' is not optional, value can never be nil}}
nil == Int.self // expected-error {{binary operator '==' cannot be applied to operands}}
// expected-note @-1 {{overloads for '==' exist with these partially matching parameter lists}}
Int.self != nil // expected-error {{value of type 'Int.Type' can never be nil, comparison isn't allowed}}
Int.self != nil // expected-error {{type 'Int.Type' is not optional, value can never be nil}}
nil != Int.self // expected-error {{binary operator '!=' cannot be applied to operands}}
// expected-note @-1 {{overloads for '!=' exist with these partially matching parameter lists}}

Expand Down