Skip to content

[Diagnostics] Improve diagnostics for optional/integer as boolean condition #29628

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 2 commits into from
Feb 4, 2020
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
5 changes: 4 additions & 1 deletion include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -3177,7 +3177,10 @@ NOTE(note_explicitly_compare_cftypeid,none,

ERROR(optional_used_as_boolean,none,
"optional type %0 cannot be used as a boolean; "
"test for '!= nil' instead", (Type))
"test for '%select{!|=}1= nil' instead", (Type, bool))
ERROR(integer_used_as_boolean,none,
"type %0 cannot be used as a boolean; "
"test for '%select{!|=}1= 0' instead", (Type, bool))

ERROR(interpolation_missing_proto,none,
"string interpolation requires the protocol 'ExpressibleByStringInterpolation' to be defined",
Expand Down
1 change: 1 addition & 0 deletions include/swift/AST/KnownIdentifiers.def
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ IDENTIFIER_WITH_NAME(NativeClassLayout, "_NativeClass")
// Operators
IDENTIFIER_WITH_NAME(MatchOperator, "~=")
IDENTIFIER_WITH_NAME(EqualsOperator, "==")
IDENTIFIER_WITH_NAME(NegationOperator, "!")
IDENTIFIER_WITH_NAME(derived_enum_equals, "__derived_enum_equals")
IDENTIFIER_WITH_NAME(derived_struct_equals, "__derived_struct_equals")

Expand Down
1 change: 1 addition & 0 deletions include/swift/AST/KnownProtocols.def
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ PROTOCOL_(ErrorCodeProtocol)
PROTOCOL(OptionSet)
PROTOCOL(CaseIterable)
PROTOCOL(SIMDScalar)
PROTOCOL(BinaryInteger)

PROTOCOL_(BridgedNSError)
PROTOCOL_(BridgedStoredNSError)
Expand Down
1 change: 1 addition & 0 deletions lib/IRGen/GenMeta.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4370,6 +4370,7 @@ SpecialProtocol irgen::getSpecialProtocolID(ProtocolDecl *P) {
case KnownProtocolKind::CaseIterable:
case KnownProtocolKind::Comparable:
case KnownProtocolKind::SIMDScalar:
case KnownProtocolKind::BinaryInteger:
case KnownProtocolKind::ObjectiveCBridgeable:
case KnownProtocolKind::DestructorSafeContainer:
case KnownProtocolKind::SwiftNewtypeWrapper:
Expand Down
66 changes: 61 additions & 5 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2306,14 +2306,68 @@ bool ContextualFailure::diagnoseConversionToBool() const {
return true;
}

// Determine if the boolean negation operator was applied to the anchor. This
// upwards traversal of the AST is somewhat fragile, but enables much better
// diagnostics if someone attempts to use an optional or integer as a boolean
// condition.
SourceLoc notOperatorLoc;
if (Expr *parent = findParentExpr(getAnchor())) {
if (isa<ParenExpr>(parent) && parent->isImplicit()) {
if ((parent = findParentExpr(parent))) {
auto parentOperatorApplication = dyn_cast<PrefixUnaryExpr>(parent);
if (parentOperatorApplication) {
auto operatorRefExpr =
dyn_cast<DeclRefExpr>(parentOperatorApplication->getFn());
if (operatorRefExpr && operatorRefExpr->getDecl()->getBaseName() ==
getASTContext().Id_NegationOperator) {
notOperatorLoc = operatorRefExpr->getLoc();
}
}
}
}
}

// If we're trying to convert something from optional type to Bool, then a
// comparison against nil was probably expected.
// TODO: It would be nice to handle "!x" --> x == false, but we have no way
// to get to the parent expr at present.
auto fromType = getFromType();
if (fromType->getOptionalObjectType()) {
StringRef prefix = "((";
StringRef suffix = ") != nil)";
StringRef suffix;
if (notOperatorLoc.isValid())
suffix = ") == nil)";
else
suffix = ") != nil)";

// Check if we need the inner parentheses.
// Technically we only need them if there's something in 'expr' with
// lower precedence than '!=', but the code actually comes out nicer
// in most cases with parens on anything non-trivial.
if (expr->canAppendPostfixExpression()) {
prefix = prefix.drop_back();
suffix = suffix.drop_front();
}
// FIXME: The outer parentheses may be superfluous too.

emitDiagnostic(expr->getLoc(), diag::optional_used_as_boolean, fromType,
notOperatorLoc.isValid())
.fixItInsert(expr->getStartLoc(), prefix)
.fixItInsertAfter(expr->getEndLoc(), suffix)
.fixItRemove(notOperatorLoc);
return true;
}

// If we're trying to convert something from optional type to an integer, then
// a comparison against nil was probably expected.
auto &cs = getConstraintSystem();
if (conformsToKnownProtocol(cs, fromType, KnownProtocolKind::BinaryInteger) &&
conformsToKnownProtocol(cs, fromType,
KnownProtocolKind::ExpressibleByIntegerLiteral)) {
StringRef prefix = "((";
StringRef suffix;
if (notOperatorLoc.isValid())
suffix = ") == 0)";
else
suffix = ") != 0)";

// Check if we need the inner parentheses.
// Technically we only need them if there's something in 'expr' with
Expand All @@ -2325,9 +2379,11 @@ bool ContextualFailure::diagnoseConversionToBool() const {
}
// FIXME: The outer parentheses may be superfluous too.

emitDiagnostic(expr->getLoc(), diag::optional_used_as_boolean, fromType)
emitDiagnostic(expr->getLoc(), diag::integer_used_as_boolean, fromType,
notOperatorLoc.isValid())
.fixItInsert(expr->getStartLoc(), prefix)
.fixItInsertAfter(expr->getEndLoc(), suffix);
.fixItInsertAfter(expr->getEndLoc(), suffix)
.fixItRemove(notOperatorLoc);
return true;
}

Expand Down
8 changes: 4 additions & 4 deletions test/ClangImporter/private_frameworks_modules.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import PrivateAsParallel_Private
#error("OLD or NEW must be defined")
#endif

let _: Bool = PSGlobal // expected-error {{cannot convert value of type 'Int32'}}
let _: Bool = PSPrivateGlobal // expected-error {{cannot convert value of type 'Int32'}}
let _: Bool = PPGlobal // expected-error {{cannot convert value of type 'Int32'}}
let _: Bool = PPPrivateGlobal // expected-error {{cannot convert value of type 'Int32'}}
let _: Bool = PSGlobal // expected-error {{type 'Int32' cannot be used as a boolean}}
let _: Bool = PSPrivateGlobal // expected-error {{type 'Int32' cannot be used as a boolean}}
let _: Bool = PPGlobal // expected-error {{type 'Int32' cannot be used as a boolean}}
let _: Bool = PPPrivateGlobal // expected-error {{type 'Int32' cannot be used as a boolean}}
8 changes: 4 additions & 4 deletions test/Constraints/diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,10 @@ func r17224804(_ monthNumber : Int) {

// <rdar://problem/17020197> QoI: Operand of postfix '!' should have optional type; type is 'Int?'
func r17020197(_ x : Int?, y : Int) {
if x! { } // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
if x! { } // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}

// <rdar://problem/12939553> QoI: diagnostic for using an integer in a condition is utterly terrible
if y {} // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
if y {} // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
}

// <rdar://problem/20714480> QoI: Boolean expr not treated as Bool type when function return type is different
Expand Down Expand Up @@ -634,11 +634,11 @@ func r18397777(_ d : r21447318?) {
if d { // expected-error {{optional type 'r21447318?' cannot be used as a boolean; test for '!= nil' instead}} {{6-6=(}} {{7-7= != nil)}}
}

if !d { // expected-error {{optional type 'r21447318?' cannot be used as a boolean; test for '!= nil' instead}} {{7-7=(}} {{8-8= != nil)}}
if !d { // expected-error {{optional type 'r21447318?' cannot be used as a boolean; test for '== nil' instead}} {{6-7=}} {{7-7=(}} {{8-8= == nil)}}

}

if !Optional(c) { // expected-error {{optional type 'Optional<r21447318>' cannot be used as a boolean; test for '!= nil' instead}} {{7-7=(}} {{18-18= != nil)}}
if !Optional(c) { // expected-error {{optional type 'Optional<r21447318>' cannot be used as a boolean; test for '== nil' instead}} {{6-7=}} {{7-7=(}} {{18-18= == nil)}}
}
}

Expand Down
21 changes: 17 additions & 4 deletions test/Constraints/fixes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,24 @@ var ciuo: C! = nil
if co {} // expected-error{{optional type 'C?' cannot be used as a boolean; test for '!= nil' instead}}{{4-4=(}} {{6-6= != nil)}}
if ciuo {} // expected-error{{optional type 'C?' cannot be used as a boolean; test for '!= nil' instead}}{{4-4=(}} {{8-8= != nil)}}
co ? true : false // expected-error{{optional type 'C?' cannot be used as a boolean; test for '!= nil' instead}}{{1-1=(}} {{3-3= != nil)}}
!co ? false : true // expected-error{{optional type 'C?' cannot be used as a boolean; test for '!= nil' instead}}{{2-2=(}} {{4-4= != nil)}}
!co ? false : true // expected-error{{optional type 'C?' cannot be used as a boolean; test for '== nil' instead}} {{1-2=}} {{2-2=(}} {{4-4= == nil)}}
ciuo ? true : false // expected-error{{optional type 'C?' cannot be used as a boolean; test for '!= nil' instead}}{{1-1=(}} {{5-5= != nil)}}
!ciuo ? false : true // expected-error{{optional type 'C?' cannot be used as a boolean; test for '!= nil' instead}}{{2-2=(}} {{6-6= != nil)}}
!co // expected-error{{optional type 'C?' cannot be used as a boolean; test for '!= nil' instead}}{{2-2=(}} {{4-4= != nil)}}
!ciuo // expected-error{{optional type 'C?' cannot be used as a boolean; test for '!= nil' instead}}{{2-2=(}} {{6-6= != nil)}}
!ciuo ? false : true // expected-error{{optional type 'C?' cannot be used as a boolean; test for '== nil' instead}} {{1-2=}} {{2-2=(}} {{6-6= == nil)}}
!co // expected-error{{optional type 'C?' cannot be used as a boolean; test for '== nil' instead}} {{1-2=}} {{2-2=(}} {{4-4= == nil)}}
!ciuo // expected-error{{optional type 'C?' cannot be used as a boolean; test for '== nil' instead}} {{1-2=}} {{2-2=(}} {{6-6= == nil)}}

// Used an integer in a conditional expression
var n1: Int = 1
var n2: UInt8 = 0
var n3: Int16 = 2

if n1 {} // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}} {{4-4=(}} {{6-6= != 0)}}
if !n1 {} // expected-error {{type 'Int' cannot be used as a boolean; test for '== 0' instead}} {{4-5=}} {{5-5=(}} {{7-7= == 0)}}
n1 ? true : false // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}} {{1-1=(}} {{3-3= != 0)}}
!n1 ? false : true // expected-error {{type 'Int' cannot be used as a boolean; test for '== 0' instead}} {{1-2=}} {{2-2=(}} {{4-4= == 0)}}
!n1 // expected-error {{type 'Int' cannot be used as a boolean; test for '== 0' instead}} {{1-2=}} {{2-2=(}} {{4-4= == 0)}}
if n2 {} // expected-error {{type 'UInt8' cannot be used as a boolean; test for '!= 0' instead}} {{4-4=(}} {{6-6= != 0)}}
!n3 // expected-error {{type 'Int16' cannot be used as a boolean; test for '== 0' instead}} {{1-2=}} {{2-2=(}} {{4-4= == 0)}}

// Forgotten ! or ?
var someInt = co.a // expected-error{{value of optional type 'C?' must be unwrapped to refer to member 'a' of wrapped base type 'C'}}
Expand Down
6 changes: 3 additions & 3 deletions test/Constraints/if_expr.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ useDouble(c)
useDouble(d)

var z = true ? a : b // expected-error{{result values in '? :' expression have mismatching types 'Int' and 'Double'}}
var _ = a ? b : b // expected-error{{cannot convert value of type 'Int' to expected condition type 'Bool'}}
var _ = a ? b : b // expected-error{{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}



Expand All @@ -51,9 +51,9 @@ useD1(i) // expected-error{{cannot convert value of type 'B' to expected argumen
useD2(i) // expected-error{{cannot convert value of type 'B' to expected argument type 'D2'}}

var x = true ? 1 : 0
var y = 22 ? 1 : 0 // expected-error{{cannot convert value of type 'Int' to expected condition type 'Bool'}}
var y = 22 ? 1 : 0 // expected-error{{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}

_ = x ? x : x // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
_ = x ? x : x // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
_ = true ? x : 1.2 // expected-error {{result values in '? :' expression have mismatching types 'Int' and 'Double'}}

_ = (x: true) ? true : false // expected-error {{cannot convert value of type '(x: Bool)' to expected condition type 'Bool'}}
Expand Down
6 changes: 3 additions & 3 deletions test/Misc/misc_diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ let total = 15.0
let count = 7
let median = total / count // expected-error {{binary operator '/' cannot be applied to operands of type 'Double' and 'Int'}} expected-note {{overloads for '/' exist with these partially matching parameter lists:}}

if (1) {} // expected-error{{cannot convert value of type 'Int' to expected condition type 'Bool'}}
if 1 {} // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
if (1) {} // expected-error{{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
if 1 {} // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}

var a: [String] = [1] // expected-error{{cannot convert value of type 'Int' to expected element type 'String'}}
var b: Int = [1, 2, 3] // expected-error{{cannot convert value of type '[Int]' to specified type 'Int'}}
Expand All @@ -34,7 +34,7 @@ var f2: Float = 3.0
var dd: Double = f1 - f2 // expected-error{{cannot convert value of type 'Float' to specified type 'Double'}}

func f() -> Bool {
return 1 + 1 // expected-error{{cannot convert return expression of type 'Int' to return type 'Bool'}}
return 1 + 1 // expected-error{{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
}

// Test that nested diagnostics are properly surfaced.
Expand Down
2 changes: 1 addition & 1 deletion test/NameBinding/import-resolution-overload.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ ambiguousWithVar(true) // no-warning

var localVar : Bool
localVar = false
localVar = 42 // expected-error {{cannot assign value of type 'Int' to type 'Bool'}}
localVar = 42 // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
localVar(42) // expected-error {{cannot call value of non-function type 'Bool'}}
var _ : localVar // should still work

Expand Down
2 changes: 1 addition & 1 deletion test/Parse/switch.swift
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ case _ where x % 2 == 0,
x = 1
case var y where y % 2 == 0:
x = y + 1
case _ where 0: // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
case _ where 0: // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
x = 0
default:
x = 1
Expand Down
4 changes: 2 additions & 2 deletions test/Sema/pound_assert.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@

#assert(false, "error message")

#assert(123) // expected-error{{cannot convert value of type 'Int' to expected condition type 'Bool'}}
#assert(123) // expected-error{{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}

#assert(123, "error message") // expected-error{{cannot convert value of type 'Int' to expected condition type 'Bool'}}
#assert(123, "error message") // expected-error{{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
12 changes: 6 additions & 6 deletions test/expr/expressions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func basictest() {

var x4 : Bool = true
var x5 : Bool =
4 // expected-error {{cannot convert value of type 'Int' to specified type 'Bool'}}
4 // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}

//var x6 : Float = 4+5

Expand Down Expand Up @@ -295,7 +295,7 @@ func fib(_ n: Int) -> Int {

// FIXME: Should warn about integer constants being too large <rdar://problem/14070127>
var
il_a: Bool = 4 // expected-error {{cannot convert value of type 'Int' to specified type 'Bool'}}
il_a: Bool = 4 // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
var il_b: Int8
= 123123
var il_c: Int8 = 4 // ok
Expand Down Expand Up @@ -804,13 +804,13 @@ func testParenExprInTheWay() {
let x = 42

if x & 4.0 {} // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}}
// expected-error@-1 {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
// expected-error@-1 {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
if (x & 4.0) {} // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}}
// expected-error@-1 {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
// expected-error@-1 {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
if !(x & 4.0) {} // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}}
// expected-error@-1 {{cannot convert value of type 'Int' to expected argument type 'Bool'}}
// expected-error@-1 {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}

if x & x {} // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
if x & x {} // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
}

// <rdar://problem/21352576> Mixed method/property overload groups can cause a crash during constraint optimization
Expand Down
4 changes: 2 additions & 2 deletions test/stmt/statements.swift
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func funcdecl5(_ a: Int, y: Int) {
}

// This diagnostic is terrible - rdar://12939553
if x {} // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
if x {} // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}

if true {
if (B) {
Expand Down Expand Up @@ -583,7 +583,7 @@ func fn(x: Int) {
}

func bad_if() {
if 1 {} // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
if 1 {} // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}}
if (x: false) {} // expected-error {{cannot convert value of type '(x: Bool)' to expected condition type 'Bool'}}
if (x: 1) {} // expected-error {{cannot convert value of type '(x: Int)' to expected condition type 'Bool'}}
if nil {} // expected-error {{'nil' is not compatible with expected condition type 'Bool'}}
Expand Down