From 860cddfd348842d5027915d4fab317e5ef653ce7 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 8 May 2019 11:42:17 -0700 Subject: [PATCH] [ConstraintSystem] Allow arguments to be passed by value to `@autoclosure` parameters Instead of always requiring a call to be made to pass argument to `@autoclosure` parameter, it should be allowed to pass argument by value to `@autoclosure` parameter which can return a function type. ```swift func foo(_ fn: @autoclosure () -> T) {} func bar(_ fn: @autoclosure @escaping () -> Int) { foo(fn) } ``` --- lib/Sema/CSApply.cpp | 29 +++++---- lib/Sema/CSDiagnostics.cpp | 8 +++ lib/Sema/CSSimplify.cpp | 79 +++++++++++------------ lib/Sema/ConstraintSystem.cpp | 12 ++++ lib/Sema/ConstraintSystem.h | 11 ++++ test/Compatibility/attr_autoclosure.swift | 7 ++ test/attr/attr_autoclosure.swift | 29 +++++++++ 7 files changed, 123 insertions(+), 52 deletions(-) diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index d0218f38b53..4c28cef7d84 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -5678,20 +5678,27 @@ Expr *ExprRewriter::coerceCallArguments( continue; } - auto isAutoClosureArg = [](Expr *arg) -> bool { - if (auto *DRE = dyn_cast(arg)) { - if (auto *PD = dyn_cast(DRE->getDecl())) - return PD->isAutoClosure(); + Expr *convertedArg = nullptr; + auto argRequiresAutoClosureExpr = [&](const AnyFunctionType::Param ¶m, + Type argType) { + if (!param.isAutoClosure()) + return false; + + // Since it was allowed to pass function types to @autoclosure + // parameters in Swift versions < 5, it has to be handled as + // a regular function coversion by `coerceToType`. + if (isAutoClosureArgument(arg)) { + // In Swift >= 5 mode we only allow `@autoclosure` arguments + // to be used by value if parameter would return a function + // type (it just needs to get wrapped into autoclosure expr), + // otherwise argument must always form a call. + return cs.getASTContext().isSwiftVersionAtLeast(5); } - return false; + + return true; }; - Expr *convertedArg = nullptr; - // Since it was allowed to pass function types to @autoclosure - // parameters in Swift versions < 5, it has to be handled as - // a regular function coversion by `coerceToType`. - if (param.isAutoClosure() && (!argType->is() || - !isAutoClosureArg(arg))) { + if (argRequiresAutoClosureExpr(param, argType)) { assert(!param.isInOut()); // If parameter is an autoclosure, we need to make sure that: diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index 47a1152b9c5..6c0d4add8ed 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -1760,6 +1760,14 @@ bool MissingCallFailure::diagnoseAsError() { return true; } + case ConstraintLocator::FunctionResult: { + path = path.drop_back(); + if (path.back().getKind() != ConstraintLocator::AutoclosureResult) + break; + + LLVM_FALLTHROUGH; + } + case ConstraintLocator::AutoclosureResult: { auto &cs = getConstraintSystem(); auto loc = cs.getConstraintLocator(getRawAnchor(), path.drop_back(), diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 0dec1ffda3e..5012fd692ad 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -978,30 +978,6 @@ ConstraintSystem::TypeMatchResult constraints::matchCallArguments( ? ConstraintKind::OperatorArgumentConversion : ConstraintKind::ArgumentConversion); - // Check whether argument of the call at given position refers to - // parameter marked as `@autoclosure`. This function is used to - // maintain source compatibility with Swift versions < 5, - // previously examples like following used to type-check: - // - // func foo(_ x: @autoclosure () -> Int) {} - // func bar(_ y: @autoclosure () -> Int) { - // foo(y) - // } - auto isAutoClosureArg = [&](Expr *anchor, unsigned argIdx) -> bool { - assert(anchor); - - auto *argExpr = getArgumentExpr(anchor, argIdx); - if (!argExpr) - return false; - - if (auto *DRE = dyn_cast(argExpr)) { - if (auto *param = dyn_cast(DRE->getDecl())) - return param->isAutoClosure(); - } - - return false; - }; - for (unsigned paramIdx = 0, numParams = parameterBindings.size(); paramIdx != numParams; ++paramIdx){ // Skip unfulfilled parameters. There's nothing to do for them. @@ -1012,9 +988,6 @@ ConstraintSystem::TypeMatchResult constraints::matchCallArguments( const auto ¶m = params[paramIdx]; auto paramTy = param.getOldType(); - if (param.isAutoClosure()) - paramTy = paramTy->castTo()->getResult(); - // Compare each of the bound arguments for this parameter. for (auto argIdx : parameterBindings[paramIdx]) { auto loc = locator.withPathElement(LocatorPathElt:: @@ -1022,19 +995,26 @@ ConstraintSystem::TypeMatchResult constraints::matchCallArguments( paramIdx)); auto argTy = argsWithLabels[argIdx].getOldType(); - // If parameter was marked as `@autoclosure` and argument - // is itself `@autoclosure` function type in Swift < 5, - // let's fix that up by making it look like argument is - // called implicitly. - if (param.isAutoClosure() && - isAutoClosureArg(locator.getAnchor(), argIdx)) { - argTy = argTy->castTo()->getResult(); - cs.increaseScore(SK_FunctionConversion); + bool matchingAutoClosureResult = param.isAutoClosure(); + if (param.isAutoClosure()) { + auto &ctx = cs.getASTContext(); + auto *fnType = paramTy->castTo(); + auto *argExpr = getArgumentExpr(locator.getAnchor(), argIdx); - if (cs.getASTContext().isSwiftVersionAtLeast(5)) { - auto *fixLoc = cs.getConstraintLocator(loc); - if (cs.recordFix(AutoClosureForwarding::create(cs, fixLoc))) - return cs.getTypeMatchFailure(loc); + // If the argument is not marked as @autoclosure or + // this is Swift version >= 5 where forwarding is not allowed, + // argument would always be wrapped into an implicit closure + // at the end, so we can safely match against result type. + if (ctx.isSwiftVersionAtLeast(5) || !isAutoClosureArgument(argExpr)) { + // In Swift >= 5 mode there is no @autoclosure forwarding, + // so let's match result types. + paramTy = fnType->getResult(); + } else { + // Matching @autoclosure argument to @autoclosure parameter + // directly would mean introducting a function conversion + // in Swift <= 4 mode. + cs.increaseScore(SK_FunctionConversion); + matchingAutoClosureResult = false; } } @@ -1045,7 +1025,7 @@ ConstraintSystem::TypeMatchResult constraints::matchCallArguments( cs.addConstraint( subKind, argTy, paramTy, - param.isAutoClosure() + matchingAutoClosureResult ? loc.withPathElement(ConstraintLocator::AutoclosureResult) : loc, /*isFavored=*/false); @@ -2192,6 +2172,18 @@ bool ConstraintSystem::repairFailures( break; } + case ConstraintLocator::FunctionResult: { + // `apply argument` -> `arg/param compare` -> + // `@autoclosure result` -> `function result` + if (path.size() > 3) { + const auto &elt = path[path.size() - 2]; + if (elt.getKind() == ConstraintLocator::AutoclosureResult && + repairByInsertingExplicitCall(lhs, rhs)) + return true; + } + break; + } + case ConstraintLocator::AutoclosureResult: { if (repairByInsertingExplicitCall(lhs, rhs)) return true; @@ -6565,6 +6557,12 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint( return result; } + case FixKind::AutoClosureForwarding: { + if (recordFix(fix)) + return SolutionKind::Error; + return matchTypes(type1, type2, matchKind, subflags, locator); + } + case FixKind::InsertCall: case FixKind::RemoveReturn: case FixKind::RemoveAddressOf: @@ -6580,7 +6578,6 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint( case FixKind::CoerceToCheckedCast: case FixKind::RelabelArguments: case FixKind::AddConformance: - case FixKind::AutoClosureForwarding: case FixKind::RemoveUnwrap: case FixKind::DefineMemberBasedOnUse: case FixKind::AllowTypeOrInstanceMember: diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index fd18309d442..18f68d23b9f 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2666,6 +2666,18 @@ Expr *constraints::getArgumentExpr(Expr *expr, unsigned index) { return cast(argExpr)->getElement(index); } +bool constraints::isAutoClosureArgument(Expr *argExpr) { + if (!argExpr) + return false; + + if (auto *DRE = dyn_cast(argExpr)) { + if (auto *param = dyn_cast(DRE->getDecl())) + return param->isAutoClosure(); + } + + return false; +} + void ConstraintSystem::generateConstraints( SmallVectorImpl &constraints, Type type, ArrayRef choices, DeclContext *useDC, diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index 224fb1d8e69..274b6f3e6eb 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -3793,6 +3793,17 @@ Expr *simplifyLocatorToAnchor(ConstraintSystem &cs, ConstraintLocator *locator); /// wasn't of one of the kinds listed above. Expr *getArgumentExpr(Expr *expr, unsigned index); +// Check whether argument of the call at given position refers to +// parameter marked as `@autoclosure`. This function is used to +// maintain source compatibility with Swift versions < 5, +// previously examples like following used to type-check: +// +// func foo(_ x: @autoclosure () -> Int) {} +// func bar(_ y: @autoclosure () -> Int) { +// foo(y) +// } +bool isAutoClosureArgument(Expr *argExpr); + class DisjunctionChoice { unsigned Index; Constraint *Choice; diff --git a/test/Compatibility/attr_autoclosure.swift b/test/Compatibility/attr_autoclosure.swift index ce26b8c9ce1..c0ac4baacb1 100644 --- a/test/Compatibility/attr_autoclosure.swift +++ b/test/Compatibility/attr_autoclosure.swift @@ -62,3 +62,10 @@ func passAutoClosureToEnumCase(_ fn: @escaping @autoclosure () -> Int) { // somewhere SILGen if `fn` doesn't have `@escaping`. let _: E = .baz(fn) // Ok } + +do { + func bar(_ fn: @autoclosure () -> (() -> Int)) {} + func foo(_ fn: @autoclosure @escaping () -> (() -> Int)) { + bar(fn) // Ok + } +} diff --git a/test/attr/attr_autoclosure.swift b/test/attr/attr_autoclosure.swift index ea5fbc573b9..2ec80ebb9a9 100644 --- a/test/attr/attr_autoclosure.swift +++ b/test/attr/attr_autoclosure.swift @@ -245,3 +245,32 @@ protocol P_47586626 { func foo(_: @autoclosure F) func bar(_: @autoclosure G) } + +func overloaded_autoclj(_: @autoclosure () -> T) {} +func overloaded_autoclj(_: @autoclosure () -> Int) {} + +func autoclosure_param_returning_func_type() { + func foo(_ fn: @autoclosure () -> (() -> Int)) {} + func generic_foo(_ fn: @autoclosure () -> T) {} + + func bar_1(_ fn: @autoclosure @escaping () -> Int) { foo(fn) } // Ok + func bar_2(_ fn: @autoclosure () -> Int) { foo(fn) } // expected-note {{parameter 'fn' is implicitly non-escaping}} + // expected-error@-1 {{using non-escaping parameter 'fn' in a context expecting an @escaping closure}} + func baz_1(_ fn: @autoclosure @escaping () -> Int) { generic_foo(fn) } // Ok (T is inferred as () -> Int) + func baz_2(_ fn: @autoclosure @escaping () -> Int) { generic_foo(fn()) } // Ok (T is inferred as Int) + func baz_3(_ fn: @autoclosure () -> Int) { generic_foo(fn) } // Fails because fn is not marked as @escaping + // expected-error@-1 {{converting non-escaping value to 'T' may allow it to escape}} + + // Let's make sure using `fn` as value works fine in presence of overloading + func biz_1(_ fn: @autoclosure @escaping () -> Int) { overloaded_autoclj(fn) } // Ok + func biz_2(_ fn: @autoclosure @escaping () -> Int) { overloaded_autoclj(fn()) } // Ok + func biz_3(_ fn: @autoclosure () -> Int) { overloaded_autoclj(fn) } // Fails because fn is not marked as @escaping + // expected-error@-1 {{add () to forward @autoclosure parameter}} {{67-67=()}} + + func fiz(_: @autoclosure () -> (() -> Int)) {} + + func biz_4(_ fn: @autoclosure @escaping () -> (() -> Int)) { fiz(fn) } // Can't forward in Swift >= 5 mode + // expected-error@-1 {{add () to forward @autoclosure parameter}} {{70-70=()}} + func biz_5(_ fn: @escaping () -> (() -> Int)) { fiz(fn) } // Can't forward in Swift >= 5 mode + // expected-error@-1 {{add () to forward @autoclosure parameter}} {{57-57=()}} +}