Skip to content

[SE-0279] Add support for an unbraced syntax for multiple trailing closures #31052

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 37 commits into from
May 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
0c9a37f
[AST] Extend TupleExpr to support multiple trailing closures
xedin Feb 7, 2020
4650091
[AST] Remove unused "trailing closure" argument from `totalSizeToAllo…
xedin Feb 7, 2020
5cea9b9
[AST] Add support for multiple trailing closures to the parser/expres…
xedin Feb 8, 2020
45ac1bc
[Parser] Adjust `parseExprList` to return multiple trailing closures
xedin Feb 10, 2020
07cb6b8
[Parse] Account that there could be multiple trailing closures which …
xedin Feb 10, 2020
d06126d
[Parser] Add support for multiple trailing closures syntax
xedin Feb 10, 2020
93c0d85
[Parser] NFC: Move multiple trailing closures tests into a separate file
xedin Mar 3, 2020
ed5b695
[Parse] Introduce tailored diagnostics for an invalid trailing closur…
xedin Mar 3, 2020
08a29e8
[Parser] Check for first label in trailing closure block without back…
xedin Mar 4, 2020
c8a8b89
[Parser] Propagate status information from each closure in the traili…
xedin Mar 4, 2020
150df80
[Syntax] Add declaration for multiple trailing closure
rintaro Mar 4, 2020
4c4990e
Multiple trailing closure fixes
Mar 14, 2020
c5c8c58
Add indentation support for multiple trailing closures.
Mar 14, 2020
d6dcd44
[Parse] Report "success" if '}' is found for multiple trailing closures
rintaro Mar 13, 2020
6bd20ca
[Parse] Allow multiple trailing closures to be delimited by `_:`
xedin Mar 18, 2020
fd68f09
[CodeCompletion] Completion inside multiple trailing closure
rintaro Mar 17, 2020
a518e75
WIP for a different syntax for multiple trailing closures
rjmccall Apr 9, 2020
882035f
Implement the conservative option for typechecking multiple trailing …
rjmccall Apr 9, 2020
67838cd
Call out another bad diagnostic, delete a redundant test.
rjmccall Apr 10, 2020
015d838
Claim trailing closure arguments like we used to in error cases.
rjmccall Apr 15, 2020
a2fac90
Revise test for the new multiple-closure syntax.
rjmccall Apr 15, 2020
1e1e7f3
XFAIL a pair of tooling tests that are specific to multiple trailing …
rjmccall Apr 15, 2020
8df09a0
Remove unnecessary check for a single closure.
rjmccall Apr 15, 2020
2e094a5
[Syntax] Update for braceless multiple trailing closure syntax
rintaro Apr 14, 2020
1cbb1e7
[CodeCompletion] Update for braceless multiple trailing closure
rintaro Apr 16, 2020
938c84f
Fix test.
rjmccall Apr 16, 2020
40836a0
Resolve an ambiguity with the multiple-trailing-closure syntax in fav…
rjmccall Apr 16, 2020
7cb5066
[CodeCompletion] Temporarily XFAIL a test case
rintaro Apr 16, 2020
7407a80
[CodeCompletion] Postfix expr completion after trailing closures
rintaro Apr 16, 2020
c0bf473
[CodeCompletion] Fix a crash regression
rintaro Apr 17, 2020
0355a87
[Parse] Parse editor placeholder as a labeled trailng closure
rintaro Apr 20, 2020
58859f5
[SourceKit/CodeFormat] Update indentation for braceless multiple trai…
Apr 20, 2020
78b7bce
[IDE][Refactoring] Update syntactic rename to support braceless multi…
Apr 21, 2020
feaaf39
[sourcekitd-test] Make expand-placeholder iterative
benlangmuir Mar 9, 2020
2bf014d
[expand-placeholder] Add support for multiple-trailing closures
benlangmuir Mar 9, 2020
37b98af
[CodeCompletion] Pre-expand closures in argument completion
benlangmuir May 1, 2020
5597b2f
Fix for new use of CallExpr::Create
rjmccall May 6, 2020
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
11 changes: 11 additions & 0 deletions include/swift/AST/DiagnosticsParse.def
Original file line number Diff line number Diff line change
Expand Up @@ -1750,5 +1750,16 @@ ERROR(availability_query_repeated_platform, none,
ERROR(unknown_syntax_entity, PointsToFirstBadToken,
"unknown %0 syntax exists in the source", (StringRef))

//------------------------------------------------------------------------------
// MARK: multiple trailing closures diagnostics
//------------------------------------------------------------------------------
ERROR(expected_argument_label_followed_by_closure_literal,none,
"expected an argument label followed by a closure literal", ())
ERROR(expected_closure_literal,none,
"expected a closure literal", ())

ERROR(expected_multiple_closures_block_rbrace,none,
"expected '}' at the end of a trailing closures block", ())

#define UNDEFINE_DIAGNOSTIC_MACROS
#include "DefineDiagnosticMacros.h"
131 changes: 111 additions & 20 deletions include/swift/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,18 @@ namespace swift {
class KeyPathExpr;
class CaptureListExpr;

struct TrailingClosure {
Identifier Label;
SourceLoc LabelLoc;
Expr *ClosureExpr;

TrailingClosure(Expr *closure)
: TrailingClosure(Identifier(), SourceLoc(), closure) {}

TrailingClosure(Identifier label, SourceLoc labelLoc, Expr *closure)
: Label(label), LabelLoc(labelLoc), ClosureExpr(closure) {}
};

enum class ExprKind : uint8_t {
#define EXPR(Id, Parent) Id,
#define LAST_EXPR(Id) Last_Expr = Id,
Expand Down Expand Up @@ -200,10 +212,7 @@ class alignas(8) Expr {
FieldNo : 32
);

SWIFT_INLINE_BITFIELD_FULL(TupleExpr, Expr, 1+1+1+32,
/// Whether this tuple has a trailing closure.
HasTrailingClosure : 1,

SWIFT_INLINE_BITFIELD_FULL(TupleExpr, Expr, 1+1+32,
/// Whether this tuple has any labels.
HasElementNames : 1,

Expand Down Expand Up @@ -533,6 +542,11 @@ class alignas(8) Expr {
bool isSelfExprOf(const AbstractFunctionDecl *AFD,
bool sameBase = false) const;

/// Given that this is a packed argument expression of the sort that
/// would be produced from packSingleArgument, return the index of the
/// unlabeled trailing closure, if there is one.
Optional<unsigned> getUnlabeledTrailingClosureIndexOfPackedArgument() const;

/// Produce a mapping from each subexpression to its parent
/// expression, with the provided expression serving as the root of
/// the parent map.
Expand Down Expand Up @@ -1198,7 +1212,7 @@ class ObjectLiteralExpr final
ArrayRef<Identifier> argLabels,
ArrayRef<SourceLoc> argLabelLocs,
SourceLoc rParenLoc,
Expr *trailingClosure,
ArrayRef<TrailingClosure> trailingClosures,
bool implicit);

LiteralKind getLiteralKind() const {
Expand All @@ -1220,6 +1234,11 @@ class ObjectLiteralExpr final
return Bits.ObjectLiteralExpr.HasTrailingClosure;
}

/// Return the index of the unlabeled trailing closure argument.
Optional<unsigned> getUnlabeledTrailingClosureIndex() const {
return getArg()->getUnlabeledTrailingClosureIndexOfPackedArgument();
}

SourceLoc getSourceLoc() const { return PoundLoc; }
SourceRange getSourceRange() const {
return SourceRange(PoundLoc, Arg->getEndLoc());
Expand Down Expand Up @@ -1806,6 +1825,11 @@ class DynamicSubscriptExpr final
return Bits.DynamicSubscriptExpr.HasTrailingClosure;
}

/// Return the index of the unlabeled trailing closure argument.
Optional<unsigned> getUnlabeledTrailingClosureIndex() const {
return Index->getUnlabeledTrailingClosureIndexOfPackedArgument();
}

SourceLoc getLoc() const { return Index->getStartLoc(); }

SourceLoc getStartLoc() const { return getBase()->getStartLoc(); }
Expand Down Expand Up @@ -1848,7 +1872,7 @@ class UnresolvedMemberExpr final
ArrayRef<Identifier> argLabels,
ArrayRef<SourceLoc> argLabelLocs,
SourceLoc rParenLoc,
Expr *trailingClosure,
ArrayRef<TrailingClosure> trailingClosures,
bool implicit);

DeclNameRef getName() const { return Name; }
Expand All @@ -1875,6 +1899,11 @@ class UnresolvedMemberExpr final
return Bits.UnresolvedMemberExpr.HasTrailingClosure;
}

/// Return the index of the unlabeled trailing closure argument.
Optional<unsigned> getUnlabeledTrailingClosureIndex() const {
return getArgument()->getUnlabeledTrailingClosureIndexOfPackedArgument();
}

SourceLoc getLoc() const { return NameLoc.getBaseNameLoc(); }

SourceLoc getStartLoc() const { return DotLoc; }
Expand Down Expand Up @@ -2056,6 +2085,11 @@ class ParenExpr : public IdentityExpr {
/// Whether this expression has a trailing closure as its argument.
bool hasTrailingClosure() const { return Bits.ParenExpr.HasTrailingClosure; }

Optional<unsigned>
getUnlabeledTrailingClosureIndexOfPackedArgument() const {
return hasTrailingClosure() ? Optional<unsigned>(0) : None;
}

static bool classof(const Expr *E) { return E->getKind() == ExprKind::Paren; }
};

Expand All @@ -2069,6 +2103,8 @@ class TupleExpr final : public Expr,
SourceLoc LParenLoc;
SourceLoc RParenLoc;

Optional<unsigned> FirstTrailingArgumentAt;

size_t numTrailingObjects(OverloadToken<Expr *>) const {
return getNumElements();
}
Expand All @@ -2095,11 +2131,11 @@ class TupleExpr final : public Expr,
return { getTrailingObjects<SourceLoc>(), getNumElements() };
}

TupleExpr(SourceLoc LParenLoc, ArrayRef<Expr *> SubExprs,
ArrayRef<Identifier> ElementNames,
TupleExpr(SourceLoc LParenLoc, SourceLoc RParenLoc,
ArrayRef<Expr *> SubExprs,
ArrayRef<Identifier> ElementNames,
ArrayRef<SourceLoc> ElementNameLocs,
SourceLoc RParenLoc, bool HasTrailingClosure, bool Implicit,
Type Ty);
Optional<unsigned> FirstTrailingArgumentAt, bool Implicit, Type Ty);

public:
/// Create a tuple.
Expand All @@ -2111,6 +2147,15 @@ class TupleExpr final : public Expr,
SourceLoc RParenLoc, bool HasTrailingClosure,
bool Implicit, Type Ty = Type());

static TupleExpr *create(ASTContext &ctx,
SourceLoc LParenLoc,
SourceLoc RParenLoc,
ArrayRef<Expr *> SubExprs,
ArrayRef<Identifier> ElementNames,
ArrayRef<SourceLoc> ElementNameLocs,
Optional<unsigned> FirstTrailingArgumentAt,
bool Implicit, Type Ty = Type());

/// Create an empty tuple.
static TupleExpr *createEmpty(ASTContext &ctx, SourceLoc LParenLoc,
SourceLoc RParenLoc, bool Implicit);
Expand All @@ -2124,8 +2169,25 @@ class TupleExpr final : public Expr,

SourceRange getSourceRange() const;

bool hasAnyTrailingClosures() const {
return (bool) FirstTrailingArgumentAt;
}

/// Whether this expression has a trailing closure as its argument.
bool hasTrailingClosure() const { return Bits.TupleExpr.HasTrailingClosure; }
bool hasTrailingClosure() const {
return FirstTrailingArgumentAt
? *FirstTrailingArgumentAt == getNumElements() - 1
: false;
}

bool hasMultipleTrailingClosures() const {
return FirstTrailingArgumentAt ? !hasTrailingClosure() : false;
}

Optional<unsigned>
getUnlabeledTrailingClosureIndexOfPackedArgument() const {
return FirstTrailingArgumentAt;
}

/// Retrieve the elements of this tuple.
MutableArrayRef<Expr*> getElements() {
Expand All @@ -2136,8 +2198,22 @@ class TupleExpr final : public Expr,
ArrayRef<Expr*> getElements() const {
return { getTrailingObjects<Expr *>(), getNumElements() };
}

MutableArrayRef<Expr*> getTrailingElements() {
return getElements().take_back(getNumTrailingElements());
}

ArrayRef<Expr*> getTrailingElements() const {
return getElements().take_back(getNumTrailingElements());
}

unsigned getNumElements() const { return Bits.TupleExpr.NumElements; }

unsigned getNumTrailingElements() const {
return FirstTrailingArgumentAt
? getNumElements() - *FirstTrailingArgumentAt
: 0;
}

Expr *getElement(unsigned i) const {
return getElements()[i];
Expand Down Expand Up @@ -2374,7 +2450,7 @@ class SubscriptExpr final : public LookupExpr,
ArrayRef<Identifier> indexArgLabels,
ArrayRef<SourceLoc> indexArgLabelLocs,
SourceLoc rSquareLoc,
Expr *trailingClosure,
ArrayRef<TrailingClosure> trailingClosures,
ConcreteDeclRef decl = ConcreteDeclRef(),
bool implicit = false,
AccessSemantics semantics
Expand All @@ -2398,6 +2474,11 @@ class SubscriptExpr final : public LookupExpr,
return Bits.SubscriptExpr.HasTrailingClosure;
}

/// Return the index of the unlabeled trailing closure argument.
Optional<unsigned> getUnlabeledTrailingClosureIndex() const {
return getIndex()->getUnlabeledTrailingClosureIndexOfPackedArgument();
}

/// Determine whether this subscript reference should bypass the
/// ordinary accessors.
AccessSemantics getAccessSemantics() const {
Expand Down Expand Up @@ -4243,6 +4324,9 @@ class ApplyExpr : public Expr {
/// Whether this application was written using a trailing closure.
bool hasTrailingClosure() const;

/// Return the index of the unlabeled trailing closure argument.
Optional<unsigned> getUnlabeledTrailingClosureIndex() const;

static bool classof(const Expr *E) {
return E->getKind() >= ExprKind::First_ApplyExpr &&
E->getKind() <= ExprKind::Last_ApplyExpr;
Expand Down Expand Up @@ -4287,8 +4371,8 @@ class CallExpr final : public ApplyExpr,
llvm::function_ref<Type(Expr *)> getType = [](Expr *E) -> Type {
return E->getType();
}) {
return create(ctx, fn, SourceLoc(), args, argLabels, { }, SourceLoc(),
/*trailingClosure=*/nullptr, /*implicit=*/true, getType);
return create(ctx, fn, SourceLoc(), args, argLabels, {}, SourceLoc(),
/*trailingClosures=*/{}, /*implicit=*/true, getType);
}

/// Create a new call expression.
Expand All @@ -4299,11 +4383,12 @@ class CallExpr final : public ApplyExpr,
/// or which must be empty.
/// \param argLabelLocs The locations of the argument labels, whose size must
/// equal args.size() or which must be empty.
/// \param trailingClosure The trailing closure, if any.
/// \param trailingClosures The list of trailing closures, if any.
static CallExpr *create(
ASTContext &ctx, Expr *fn, SourceLoc lParenLoc, ArrayRef<Expr *> args,
ArrayRef<Identifier> argLabels, ArrayRef<SourceLoc> argLabelLocs,
SourceLoc rParenLoc, Expr *trailingClosure, bool implicit,
SourceLoc rParenLoc, ArrayRef<TrailingClosure> trailingClosures,
bool implicit,
llvm::function_ref<Type(Expr *)> getType = [](Expr *E) -> Type {
return E->getType();
});
Expand All @@ -4325,9 +4410,14 @@ class CallExpr final : public ApplyExpr,
unsigned getNumArguments() const { return Bits.CallExpr.NumArgLabels; }
bool hasArgumentLabelLocs() const { return Bits.CallExpr.HasArgLabelLocs; }

/// Whether this call with written with a trailing closure.
/// Whether this call with written with a single trailing closure.
bool hasTrailingClosure() const { return Bits.CallExpr.HasTrailingClosure; }

/// Return the index of the unlabeled trailing closure argument.
Optional<unsigned> getUnlabeledTrailingClosureIndex() const {
return getArg()->getUnlabeledTrailingClosureIndexOfPackedArgument();
}

using TrailingCallArguments::getArgumentLabels;

/// Retrieve the expression that directly represents the callee.
Expand Down Expand Up @@ -5191,7 +5281,7 @@ class KeyPathExpr : public Expr {
ArrayRef<Identifier> indexArgLabels,
ArrayRef<SourceLoc> indexArgLabelLocs,
SourceLoc rSquareLoc,
Expr *trailingClosure);
ArrayRef<TrailingClosure> trailingClosures);

/// Create an unresolved component for a subscript.
///
Expand Down Expand Up @@ -5243,7 +5333,7 @@ class KeyPathExpr : public Expr {
ArrayRef<Identifier> indexArgLabels,
ArrayRef<SourceLoc> indexArgLabelLocs,
SourceLoc rSquareLoc,
Expr *trailingClosure,
ArrayRef<TrailingClosure> trailingClosures,
Type elementType,
ArrayRef<ProtocolConformanceRef> indexHashables);

Expand Down Expand Up @@ -5631,7 +5721,8 @@ inline const SourceLoc *CollectionExpr::getTrailingSourceLocs() const {
Expr *packSingleArgument(
ASTContext &ctx, SourceLoc lParenLoc, ArrayRef<Expr *> args,
ArrayRef<Identifier> &argLabels, ArrayRef<SourceLoc> &argLabelLocs,
SourceLoc rParenLoc, Expr *trailingClosure, bool implicit,
SourceLoc rParenLoc, ArrayRef<TrailingClosure> trailingClosures,
bool implicit,
SmallVectorImpl<Identifier> &argLabelsScratch,
SmallVectorImpl<SourceLoc> &argLabelLocsScratch,
llvm::function_ref<Type(Expr *)> getType = [](Expr *E) -> Type {
Expand Down
6 changes: 2 additions & 4 deletions include/swift/AST/TrailingCallArguments.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,14 @@ class TrailingCallArguments
protected:
/// Determine the total size to allocate.
static size_t totalSizeToAlloc(ArrayRef<Identifier> argLabels,
ArrayRef<SourceLoc> argLabelLocs,
bool hasTrailingClosure) {
ArrayRef<SourceLoc> argLabelLocs) {
return TrailingObjects::template totalSizeToAlloc<Identifier, SourceLoc>(
argLabels.size(), argLabelLocs.size());
}

/// Initialize the actual call arguments.
void initializeCallArguments(ArrayRef<Identifier> argLabels,
ArrayRef<SourceLoc> argLabelLocs,
bool hasTrailingClosure) {
ArrayRef<SourceLoc> argLabelLocs) {
if (!argLabels.empty()) {
std::uninitialized_copy(argLabels.begin(), argLabels.end(),
this->template getTrailingObjects<Identifier>());
Expand Down
8 changes: 8 additions & 0 deletions include/swift/IDE/CodeCompletion.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,12 @@ class CodeCompletionStringChunk {
/// closure type if CallParameterType is a TypeAliasType.
CallParameterClosureType,

/// An expanded closure expression for the value of a parameter, including
/// the left and right braces and possible signature. The preferred
/// position to put the cursor after the completion result is inserted
/// into the editor buffer is between the braces.
CallParameterClosureExpr,

/// A placeholder for \c ! or \c ? in a call to a method found by dynamic
/// lookup.
///
Expand Down Expand Up @@ -224,6 +230,7 @@ class CodeCompletionStringChunk {
Kind == ChunkKind::DeclAttrParamKeyword ||
Kind == ChunkKind::CallParameterType ||
Kind == ChunkKind::CallParameterClosureType ||
Kind == ChunkKind::CallParameterClosureExpr ||
Kind == ChunkKind::GenericParameterName ||
Kind == ChunkKind::DynamicLookupMethodCallTail ||
Kind == ChunkKind::OptionalMethodCallTail ||
Expand Down Expand Up @@ -525,6 +532,7 @@ enum class CompletionKind {
AttributeDeclParen,
PoundAvailablePlatform,
CallArg,
LabeledTrailingClosure,
ReturnStmtExpr,
YieldStmtExpr,
ForEachSequence,
Expand Down
12 changes: 7 additions & 5 deletions include/swift/IDE/Utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ struct ResolvedLoc {
ASTWalker::ParentTy Node;
CharSourceRange Range;
std::vector<CharSourceRange> LabelRanges;
Optional<unsigned> FirstTrailingLabel;
LabelRangeType LabelType;
bool IsActive;
bool IsInSelector;
Expand Down Expand Up @@ -268,7 +269,8 @@ class NameMatcher: public ASTWalker {
bool tryResolve(ASTWalker::ParentTy Node, SourceLoc NameLoc);
bool tryResolve(ASTWalker::ParentTy Node, DeclNameLoc NameLoc, Expr *Arg);
bool tryResolve(ASTWalker::ParentTy Node, SourceLoc NameLoc, LabelRangeType RangeType,
ArrayRef<CharSourceRange> LabelLocs);
ArrayRef<CharSourceRange> LabelLocs,
Optional<unsigned> FirstTrailingLabel);
bool handleCustomAttrs(Decl *D);
Expr *getApplicableArgFor(Expr* E);

Expand Down Expand Up @@ -579,10 +581,10 @@ struct CallArgInfo {
std::vector<CallArgInfo>
getCallArgInfo(SourceManager &SM, Expr *Arg, LabelRangeEndAt EndKind);

// Get the ranges of argument labels from an Arg, either tuple or paren.
// This includes empty ranges for any unlabelled arguments, and excludes
// trailing closures.
std::vector<CharSourceRange>
// Get the ranges of argument labels from an Arg, either tuple or paren, and
// the index of the first trailing closure argument, if any. This includes empty
// ranges for any unlabelled arguments, including the first trailing closure.
std::pair<std::vector<CharSourceRange>, Optional<unsigned>>
getCallArgLabelRanges(SourceManager &SM, Expr *Arg, LabelRangeEndAt EndKind);

/// Whether a decl is defined from clang source.
Expand Down
Loading