Skip to content

Commit 79263cb

Browse files
committed
[Trailing closures] Attempt both forward and backward scans.
To better preserve source compatibility, teach the constraint solver to try both the new forward scanning rule as well as the backward scanning rule when matching a single, unlabeled trailing closure. In the extreme case, where the unlabeled trailing closure matches different parameters with the different rules, and yet both produce a potential match, introduce a disjunction to explore both possibilities. Prefer solutions that involve forward scans to those that involve backward scans, so we only use the backward scan as a fallback.
1 parent 9d19ef2 commit 79263cb

File tree

13 files changed

+554
-130
lines changed

13 files changed

+554
-130
lines changed

include/swift/AST/Attr.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ class GenericFunctionType;
5555
class LazyConformanceLoader;
5656
class LazyMemberLoader;
5757
class PatternBindingInitializer;
58-
enum class TrailingClosureMatching: uint8_t;
5958
class TrailingWhereClause;
6059

6160
/// TypeAttributes - These are attributes that may be applied to types.

lib/AST/Expr.cpp

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,15 +1271,29 @@ SourceRange TupleExpr::getSourceRange() const {
12711271
return { SourceLoc(), SourceLoc() };
12721272
} else {
12731273
// Scan backwards for a valid source loc.
1274+
bool hasSingleTrailingClosure = hasTrailingClosure();
12741275
for (Expr *expr : llvm::reverse(getElements())) {
12751276
// Default arguments are located at the start of their parent tuple, so
12761277
// skip over them.
12771278
if (isa<DefaultArgumentExpr>(expr))
12781279
continue;
1279-
end = expr->getEndLoc();
1280-
if (end.isValid()) {
1281-
break;
1280+
1281+
SourceLoc newEnd = expr->getEndLoc();
1282+
if (newEnd.isInvalid())
1283+
continue;
1284+
1285+
// There is a quirk with the backward scan logic for trailing
1286+
// closures that can cause arguments to be flipped. If there is a
1287+
// single trailing closure, only stop when the "end" point we hit comes
1288+
// after the close parenthesis (if there is one).
1289+
if (end.isInvalid() ||
1290+
end.getOpaquePointerValue() < newEnd.getOpaquePointerValue()) {
1291+
end = newEnd;
12821292
}
1293+
1294+
if (!hasSingleTrailingClosure || RParenLoc.isInvalid() ||
1295+
RParenLoc.getOpaquePointerValue() < end.getOpaquePointerValue())
1296+
break;
12831297
}
12841298
}
12851299
} else {

lib/Sema/CSApply.cpp

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5754,19 +5754,34 @@ Expr *ExprRewriter::coerceCallArguments(
57545754
AnyFunctionType::relabelParams(args, argLabels);
57555755

57565756
MatchCallArgumentListener listener;
5757-
SmallVector<ParamBinding, 4> parameterBindings;
57585757
auto unlabeledTrailingClosureIndex =
57595758
arg->getUnlabeledTrailingClosureIndexOfPackedArgument();
5760-
bool failed = constraints::matchCallArguments(args, params,
5761-
paramInfo,
5762-
unlabeledTrailingClosureIndex,
5763-
/*allowFixes=*/false, listener,
5764-
parameterBindings);
5765-
5766-
assert((matchCanFail || !failed) && "Call arguments did not match up?");
5767-
(void)failed;
5759+
5760+
// Determine the trailing closure matching rule that was applied. This
5761+
// is only relevant for explicit calls and subscripts.
5762+
auto trailingClosureMatching = TrailingClosureMatching::Forward;
5763+
{
5764+
SmallVector<LocatorPathElt, 4> path;
5765+
auto anchor = locator.getLocatorParts(path);
5766+
if (!path.empty() && path.back().is<LocatorPathElt::ApplyArgument>() &&
5767+
(anchor.isExpr(ExprKind::Call) || anchor.isExpr(ExprKind::Subscript))) {
5768+
auto locatorPtr = cs.getConstraintLocator(locator);
5769+
assert(solution.trailingClosureMatchingChoices.count(locatorPtr) == 1);
5770+
trailingClosureMatching = solution.trailingClosureMatchingChoices.find(
5771+
locatorPtr)->second;
5772+
}
5773+
}
5774+
5775+
auto callArgumentMatch = constraints::matchCallArguments(
5776+
args, params, paramInfo, unlabeledTrailingClosureIndex,
5777+
/*allowFixes=*/false, listener, trailingClosureMatching);
5778+
5779+
assert((matchCanFail || callArgumentMatch) &&
5780+
"Call arguments did not match up?");
57685781
(void)matchCanFail;
57695782

5783+
auto parameterBindings = std::move(callArgumentMatch->parameterBindings);
5784+
57705785
// We should either have parentheses or a tuple.
57715786
auto *argTuple = dyn_cast<TupleExpr>(arg);
57725787
auto *argParen = dyn_cast<ParenExpr>(arg);

lib/Sema/CSRanking.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ void ConstraintSystem::increaseScore(ScoreKind kind, unsigned value) {
4949
log << "use of an unavailable declaration";
5050
break;
5151

52+
case SK_BackwardTrailingClosure:
53+
llvm::errs() << "backward scan when matching a trailing closure";
54+
break;
55+
5256
case SK_Fix:
5357
log << "attempting to fix the source";
5458
break;

0 commit comments

Comments
 (0)