From 385fb0c6651aecc8805d054195859e054b23c31e Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 9 Aug 2019 01:09:52 -0700 Subject: [PATCH] [Diagnostics] Make force downcast fix contextual and move it to `repairFailures` This way it covers a lot more ground and doesn't conflict with other fixes. Another notable change is related to check for IUO associated with source type, that covers cases like: ```swift func foo(_ v: NSString!) -> String { return v } ``` Instead of general conversion failure check for IUO enables solver to introduce force downcast fix. --- lib/Sema/CSDiagnostics.cpp | 5 +- lib/Sema/CSDiagnostics.h | 77 +++++++++++---------- lib/Sema/CSFix.cpp | 17 +++-- lib/Sema/CSFix.h | 32 ++++----- lib/Sema/CSSimplify.cpp | 93 ++++++++++++++++++-------- test/Constraints/bridging.swift | 2 +- test/expr/cast/array_iteration.swift | 2 +- test/expr/cast/dictionary_bridge.swift | 2 + test/expr/cast/set_bridge.swift | 1 + 9 files changed, 136 insertions(+), 95 deletions(-) diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index db38df610db..924e44c16f6 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -921,8 +921,9 @@ bool MissingExplicitConversionFailure::diagnoseAsError() { if (auto *paren = dyn_cast(anchor)) anchor = paren->getSubExpr(); - auto fromType = getType(anchor)->getRValueType(); - Type toType = resolveType(ConvertingTo); + auto fromType = getFromType(); + Type toType = getToType(); + if (!toType->hasTypeRepr()) return false; diff --git a/lib/Sema/CSDiagnostics.h b/lib/Sema/CSDiagnostics.h index d4d458419ce..c1460840d27 100644 --- a/lib/Sema/CSDiagnostics.h +++ b/lib/Sema/CSDiagnostics.h @@ -544,45 +544,6 @@ public: bool diagnoseAsError() override; }; -/// Diagnose failures related attempt to implicitly convert types which -/// do not support such implicit converstion. -/// "as" or "as!" has to be specified explicitly in cases like that. -class MissingExplicitConversionFailure final : public FailureDiagnostic { - Type ConvertingTo; - -public: - MissingExplicitConversionFailure(Expr *expr, ConstraintSystem &cs, - ConstraintLocator *locator, Type toType) - : FailureDiagnostic(expr, cs, locator), ConvertingTo(toType) {} - - bool diagnoseAsError() override; - -private: - bool exprNeedsParensBeforeAddingAs(Expr *expr) { - auto *DC = getDC(); - auto &TC = getTypeChecker(); - - auto asPG = TC.lookupPrecedenceGroup( - DC, DC->getASTContext().Id_CastingPrecedence, SourceLoc()); - if (!asPG) - return true; - return exprNeedsParensInsideFollowingOperator(TC, DC, expr, asPG); - } - - bool exprNeedsParensAfterAddingAs(Expr *expr, Expr *rootExpr) { - auto *DC = getDC(); - auto &TC = getTypeChecker(); - - auto asPG = TC.lookupPrecedenceGroup( - DC, DC->getASTContext().Id_CastingPrecedence, SourceLoc()); - if (!asPG) - return true; - - return exprNeedsParensOutsideFollowingOperator(TC, DC, expr, rootExpr, - asPG); - } -}; - /// Diagnose failures related to attempting member access on optional base /// type without optional chaining or force-unwrapping it first. class MemberAccessOnOptionalBaseFailure final : public FailureDiagnostic { @@ -739,6 +700,44 @@ private: void tryComputedPropertyFixIts(Expr *expr) const; }; +/// Diagnose failures related attempt to implicitly convert types which +/// do not support such implicit converstion. +/// "as" or "as!" has to be specified explicitly in cases like that. +class MissingExplicitConversionFailure final : public ContextualFailure { +public: + MissingExplicitConversionFailure(Expr *expr, ConstraintSystem &cs, + Type fromType, Type toType, + ConstraintLocator *locator) + : ContextualFailure(expr, cs, fromType, toType, locator) {} + + bool diagnoseAsError() override; + +private: + bool exprNeedsParensBeforeAddingAs(Expr *expr) { + auto *DC = getDC(); + auto &TC = getTypeChecker(); + + auto asPG = TC.lookupPrecedenceGroup( + DC, DC->getASTContext().Id_CastingPrecedence, SourceLoc()); + if (!asPG) + return true; + return exprNeedsParensInsideFollowingOperator(TC, DC, expr, asPG); + } + + bool exprNeedsParensAfterAddingAs(Expr *expr, Expr *rootExpr) { + auto *DC = getDC(); + auto &TC = getTypeChecker(); + + auto asPG = TC.lookupPrecedenceGroup( + DC, DC->getASTContext().Id_CastingPrecedence, SourceLoc()); + if (!asPG) + return true; + + return exprNeedsParensOutsideFollowingOperator(TC, DC, expr, rootExpr, + asPG); + } +}; + /// Diagnose failures related to passing value of some type /// to `inout` or pointer parameter, without explicitly specifying `&`. class MissingAddressOfFailure final : public ContextualFailure { diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp index 3837a27e02d..0fbf0028438 100644 --- a/lib/Sema/CSFix.cpp +++ b/lib/Sema/CSFix.cpp @@ -48,21 +48,24 @@ void ConstraintFix::dump() const {print(llvm::errs()); } std::string ForceDowncast::getName() const { llvm::SmallString<16> name; - name += "force downcast (as! "; - name += DowncastTo->getString(); + name += "force downcast ("; + name += getFromType()->getString(); + name += " as! "; + name += getToType()->getString(); name += ")"; return name.c_str(); } bool ForceDowncast::diagnose(Expr *expr, bool asNote) const { - MissingExplicitConversionFailure failure(expr, getConstraintSystem(), - getLocator(), DowncastTo); + auto &cs = getConstraintSystem(); + MissingExplicitConversionFailure failure(expr, cs, getFromType(), getToType(), + getLocator()); return failure.diagnose(asNote); } -ForceDowncast *ForceDowncast::create(ConstraintSystem &cs, Type toType, - ConstraintLocator *locator) { - return new (cs.getAllocator()) ForceDowncast(cs, toType, locator); +ForceDowncast *ForceDowncast::create(ConstraintSystem &cs, Type fromType, + Type toType, ConstraintLocator *locator) { + return new (cs.getAllocator()) ForceDowncast(cs, fromType, toType, locator); } bool ForceOptional::diagnose(Expr *root, bool asNote) const { diff --git a/lib/Sema/CSFix.h b/lib/Sema/CSFix.h index 1408b06681c..1189ce08ed6 100644 --- a/lib/Sema/CSFix.h +++ b/lib/Sema/CSFix.h @@ -243,22 +243,6 @@ protected: ConstraintSystem &getConstraintSystem() const { return CS; } }; -/// Append 'as! T' to force a downcast to the specified type. -class ForceDowncast final : public ConstraintFix { - Type DowncastTo; - - ForceDowncast(ConstraintSystem &cs, Type toType, ConstraintLocator *locator) - : ConstraintFix(cs, FixKind::ForceDowncast, locator), DowncastTo(toType) { - } - -public: - std::string getName() const override; - bool diagnose(Expr *root, bool asNote = false) const override; - - static ForceDowncast *create(ConstraintSystem &cs, Type toType, - ConstraintLocator *locator); -}; - /// Introduce a '!' to force an optional unwrap. class ForceOptional final : public ConstraintFix { Type BaseType; @@ -510,6 +494,22 @@ public: ConstraintLocator *locator); }; +/// Append 'as! T' to force a downcast to the specified type. +class ForceDowncast final : public ContextualMismatch { + ForceDowncast(ConstraintSystem &cs, Type fromType, Type toType, + ConstraintLocator *locator) + : ContextualMismatch(cs, FixKind::ForceDowncast, fromType, toType, + locator) {} + +public: + std::string getName() const override; + + bool diagnose(Expr *root, bool asNote = false) const override; + + static ForceDowncast *create(ConstraintSystem &cs, Type fromType, Type toType, + ConstraintLocator *locator); +}; + /// Introduce a '&' to take the address of an lvalue. class AddAddressOf final : public ContextualMismatch { AddAddressOf(ConstraintSystem &cs, Type argTy, Type paramTy, diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index a5f78af144e..61551dfc30d 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -2186,6 +2186,58 @@ static ConstraintFix *fixPropertyWrapperFailure( return nullptr; } +static bool canBridgeThroughCast(ConstraintSystem &cs, Type fromType, + Type toType) { + // If we have a value of type AnyObject that we're trying to convert to + // a class, force a downcast. + // FIXME: Also allow types bridged through Objective-C classes. + if (fromType->isAnyObject() && toType->getClassOrBoundGenericClass()) + return true; + + auto &TC = cs.getTypeChecker(); + auto bridged = TC.getDynamicBridgedThroughObjCClass(cs.DC, fromType, toType); + if (!bridged) + return false; + + // Note: don't perform this recovery for NSNumber; + if (auto classType = bridged->getAs()) { + SmallString<16> scratch; + if (classType->getDecl()->isObjC() && + classType->getDecl()->getObjCRuntimeName(scratch) == "NSNumber") + return false; + } + + return true; +} + +static bool +repairViaBridgingCast(ConstraintSystem &cs, Type fromType, Type toType, + SmallVectorImpl &conversionsOrFixes, + ConstraintLocatorBuilder locator) { + auto objectType1 = fromType->getOptionalObjectType(); + auto objectType2 = toType->getOptionalObjectType(); + + if (objectType1 && !objectType2) { + auto *anchor = locator.trySimplifyToExpr(); + if (!anchor) + return false; + + if (auto *overload = cs.findSelectedOverloadFor(anchor)) { + auto *decl = overload->Choice.getDeclOrNull(); + if (decl && + decl->getAttrs().hasAttribute()) + fromType = objectType1; + } + } + + if (!canBridgeThroughCast(cs, fromType, toType)) + return false; + + conversionsOrFixes.push_back(ForceDowncast::create( + cs, fromType, toType, cs.getConstraintLocator(locator))); + return true; +} + /// Attempt to repair typing failures and record fixes if needed. /// \return true if at least some of the failures has been repaired /// successfully, which allows type matcher to continue. @@ -2301,6 +2353,9 @@ bool ConstraintSystem::repairFailures( if (repairByAnyToAnyObjectCast(lhs, rhs)) return true; + + if (repairViaBridgingCast(*this, lhs, rhs, conversionsOrFixes, locator)) + return true; } return false; @@ -2321,7 +2376,10 @@ bool ConstraintSystem::repairFailures( case ConstraintLocator::ApplyArgToParam: { auto loc = getConstraintLocator(locator); if (repairByInsertingExplicitCall(lhs, rhs)) - return true; + break; + + if (repairViaBridgingCast(*this, lhs, rhs, conversionsOrFixes, locator)) + break; if (lhs->getOptionalObjectType() && !rhs->getOptionalObjectType()) { conversionsOrFixes.push_back( @@ -2444,10 +2502,13 @@ bool ConstraintSystem::repairFailures( } if (repairByInsertingExplicitCall(lhs, rhs)) - return true; + break; if (repairByAnyToAnyObjectCast(lhs, rhs)) - return true; + break; + + if (repairViaBridgingCast(*this, lhs, rhs, conversionsOrFixes, locator)) + break; // If both types are key path, the only differences // between them are mutability and/or root, value type mismatch. @@ -3305,32 +3366,6 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind, } } - // If we have a value of type AnyObject that we're trying to convert to - // a class, force a downcast. - // FIXME: Also allow types bridged through Objective-C classes. - if (objectType1->isAnyObject() && - type2->getClassOrBoundGenericClass()) { - conversionsOrFixes.push_back( - ForceDowncast::create(*this, type2, getConstraintLocator(locator))); - } - - // If we could perform a bridging cast, try it. - if (auto bridged = - TC.getDynamicBridgedThroughObjCClass(DC, objectType1, type2)) { - // Note: don't perform this recovery for NSNumber; - bool useFix = true; - if (auto classType = bridged->getAs()) { - SmallString<16> scratch; - if (classType->getDecl()->isObjC() && - classType->getDecl()->getObjCRuntimeName(scratch) == "NSNumber") - useFix = false; - } - - if (useFix) - conversionsOrFixes.push_back( - ForceDowncast::create(*this, type2, getConstraintLocator(locator))); - } - if (!type1->is() && type2->is()) { // If we have a concrete type that's an rvalue, "fix" it. conversionsOrFixes.push_back( diff --git a/test/Constraints/bridging.swift b/test/Constraints/bridging.swift index 5c2f4241612..5c65f80eb53 100644 --- a/test/Constraints/bridging.swift +++ b/test/Constraints/bridging.swift @@ -296,7 +296,7 @@ func rdar20029786(_ ns: NSString?) { let s3: NSString? = "str" as String? // expected-error {{cannot convert value of type 'String?' to specified type 'NSString?'}}{{39-39= as NSString?}} - var s4: String = ns ?? "str" // expected-error{{cannot convert value of type 'NSString' to specified type 'String'}}{{31-31= as String}} + var s4: String = ns ?? "str" // expected-error{{'NSString' is not implicitly convertible to 'String'; did you mean to use 'as' to explicitly convert?}} {{20-20=(}} {{31-31=) as String}} var s5: String = (ns ?? "str") as String // fixed version } diff --git a/test/expr/cast/array_iteration.swift b/test/expr/cast/array_iteration.swift index 61e98e5663e..2405a76f325 100644 --- a/test/expr/cast/array_iteration.swift +++ b/test/expr/cast/array_iteration.swift @@ -16,7 +16,7 @@ for view in rootView.subviews as! [View] { // expected-warning{{immutable value doFoo() } -for view:View in rootView.subviews { // expected-error{{'AnyObject' is not convertible to 'View'}} +for view:View in rootView.subviews { // expected-error{{cannot convert sequence element type 'AnyObject' to expected type 'View'}} doFoo() } diff --git a/test/expr/cast/dictionary_bridge.swift b/test/expr/cast/dictionary_bridge.swift index 8bfcbe4bdf6..a2e7b4010fc 100644 --- a/test/expr/cast/dictionary_bridge.swift +++ b/test/expr/cast/dictionary_bridge.swift @@ -80,7 +80,9 @@ func testUpcastBridge() { dictOB = dictBB as [ObjC: BridgedToObjC] dictBB = dictBO // expected-error{{cannot assign value of type '[BridgedToObjC : ObjC]' to type '[BridgedToObjC : BridgedToObjC]'}} + // expected-note@-1 {{arguments to generic parameter 'Value' ('ObjC' and 'BridgedToObjC') are expected to be equal}} dictBB = dictOB // expected-error{{cannot assign value of type '[ObjC : BridgedToObjC]' to type '[BridgedToObjC : BridgedToObjC]'}} + // expected-note@-1 {{arguments to generic parameter 'Key' ('ObjC' and 'BridgedToObjC') are expected to be equal}} dictDO = dictBB // expected-error{{cannot assign value of type '[BridgedToObjC : BridgedToObjC]' to type '[DerivesObjC : ObjC]'}} //expected-note@-1 {{arguments to generic parameter 'Key' ('BridgedToObjC' and 'DerivesObjC') are expected to be equal}} diff --git a/test/expr/cast/set_bridge.swift b/test/expr/cast/set_bridge.swift index b91fc37fe57..2ba6d0dfd9c 100644 --- a/test/expr/cast/set_bridge.swift +++ b/test/expr/cast/set_bridge.swift @@ -59,6 +59,7 @@ func testUpcastBridge() { // Upcast object to bridged type setB = setO // expected-error{{cannot assign value of type 'Set' to type 'Set'}} + // expected-note@-1 {{arguments to generic parameter 'Element' ('ObjC' and 'BridgedToObjC') are expected to be equal}} // Failed upcast setD = setB // expected-error{{cannot assign value of type 'Set' to type 'Set'}}