Skip to content

Allow separators to be omitted from lists. #22714

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.

Sign up for GitHub

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

Closed
wants to merge 17 commits into from
Closed

Allow separators to be omitted from lists. #22714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
83ede8b
WIP: Allow separators to be omitted from lists.
nate-chandler Feb 19, 2019
0222aaa
Basic test coverage.
nate-chandler Feb 20, 2019
5b7c04b
Allowed omission from function calls and tuples.
nate-chandler Mar 11, 2019
57ea012
Added tests on initializers.
nate-chandler Mar 11, 2019
dd5a874
Corrected argument capitalization.
nate-chandler Mar 11, 2019
ab31290
Add tests for indentation, swift-syntax for optional commas
benlangmuir Mar 19, 2019
0d0d843
Add code-completion tests for optional commas
benlangmuir Mar 22, 2019
917d989
Corrected expected error messages in Parse/invalid.swift.
nate-chandler Mar 25, 2019
5cf817d
Stmts/decls can start line if separators omitted.
nate-chandler Mar 29, 2019
39d6296
Updated tests with revised diagnostics.
nate-chandler Apr 1, 2019
1fdfe90
Bail out of expr sequence parsing if peek shows :.
nate-chandler Apr 1, 2019
6b393ae
Updated new parseExprList call.
nate-chandler Apr 1, 2019
da4873b
Fix test after change to contextual type
benlangmuir Apr 1, 2019
acdd16f
Parsed keywords as argument labels correctly, fixing diagnostic regre…
nate-chandler Apr 5, 2019
faecb8d
Lookahead in parseList, not isStartOf{Decl,Stmt}.
nate-chandler Apr 5, 2019
4b8c2f9
Unified two separate conditional blocks into one.
nate-chandler Apr 5, 2019
2adc41d
Fixed whitespace.
nate-chandler Apr 8, 2019
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
8 changes: 5 additions & 3 deletions include/swift/Parse/Parser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -780,8 +780,8 @@ class Parser {

/// Parse a comma separated list of some elements.
ParserStatus parseList(tok RightK, SourceLoc LeftLoc, SourceLoc &RightLoc,
bool AllowSepAfterLast, Diag<> ErrorDiag,
syntax::SyntaxKind Kind,
bool AllowSepAfterLast, bool AllowSepOmission,
Diag<> ErrorDiag, syntax::SyntaxKind Kind,
llvm::function_ref<ParserStatus()> callback);

void consumeTopLevelDecl(ParserPosition BeginParserPosition,
Expand Down Expand Up @@ -1377,13 +1377,15 @@ class Parser {
SourceLoc &inLoc);

Expr *parseExprAnonClosureArg();
ParserResult<Expr> parseExprList(tok LeftTok, tok RightTok,
ParserResult<Expr> parseExprList(tok LeftTok, tok RightTok,
bool AllowSepOmission,
syntax::SyntaxKind Kind);

/// Parse an expression list, keeping all of the pieces separated.
ParserStatus parseExprList(tok leftTok, tok rightTok,
bool isPostfix,
bool isExprBasic,
bool allowSepOmission,
SourceLoc &leftLoc,
SmallVectorImpl<Expr *> &exprs,
SmallVectorImpl<Identifier> &exprLabels,
Expand Down
1 change: 1 addition & 0 deletions lib/Parse/ParseDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1894,6 +1894,7 @@ bool Parser::parseDeclAttribute(DeclAttributes &Attributes, SourceLoc AtLoc) {
ParserStatus status = parseExprList(tok::l_paren, tok::r_paren,
/*isPostfix=*/false,
/*isExprBasic=*/true,
/*allowSepOmission=*/false,
lParenLoc, args, argLabels,
argLabelLocs,
rParenLoc,
Expand Down
31 changes: 24 additions & 7 deletions lib/Parse/ParseExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,10 @@ ParserResult<Expr> Parser::parseExprSequence(Diag<> Message,
break;

parse_operator:
if (peekToken().is(tok::colon))
// If we're about to hit a colon, we aren't parsing a binary operator, so
// we're done.
goto done;
switch (Tok.getKind()) {
case tok::oper_binary_spaced:
case tok::oper_binary_unspaced: {
Expand Down Expand Up @@ -920,6 +924,7 @@ ParserResult<Expr> Parser::parseExprSuper(bool isExprBasic) {

ParserStatus status = parseExprList(tok::l_square, tok::r_square,
/*isPostfix=*/true, isExprBasic,
/*allowSepOmission=*/true,
lSquareLoc, indexArgs, indexArgLabels,
indexArgLabelLocs,
rSquareLoc,
Expand Down Expand Up @@ -1237,9 +1242,9 @@ Parser::parseExprPostfixSuffix(ParserResult<Expr> Result, bool isExprBasic,

ParserStatus status = parseExprList(
tok::l_square, tok::r_square,
/*isPostfix=*/true, isExprBasic, lSquareLoc, indexArgs,
indexArgLabels, indexArgLabelLocs, rSquareLoc, trailingClosure,
SyntaxKind::FunctionCallArgumentList);
/*isPostfix=*/true, isExprBasic, /*allowSepOmission=*/true,
lSquareLoc, indexArgs, indexArgLabels, indexArgLabelLocs, rSquareLoc,
trailingClosure, SyntaxKind::FunctionCallArgumentList);
Result = makeParserResult(
status | Result,
SubscriptExpr::create(Context, Result.get(), lSquareLoc, indexArgs,
Expand Down Expand Up @@ -1647,6 +1652,7 @@ ParserResult<Expr> Parser::parseExprPrimary(Diag<> ID, bool isExprBasic) {

ParserStatus status = parseExprList(tok::l_paren, tok::r_paren,
/*isPostfix=*/true, isExprBasic,
/*allowSepOmission=*/true,
lParenLoc, args, argLabels,
argLabelLocs,
rParenLoc,
Expand Down Expand Up @@ -1701,7 +1707,7 @@ ParserResult<Expr> Parser::parseExprPrimary(Diag<> ID, bool isExprBasic) {
// differentiation. A tuple expression node in libSyntax can have a single
// element without label.
ExprContext.setCreateSyntax(SyntaxKind::TupleExpr);
return parseExprList(tok::l_paren, tok::r_paren,
return parseExprList(tok::l_paren, tok::r_paren, /*allowSepOmission=*/true,
SyntaxKind::TupleElementList);

case tok::l_square:
Expand Down Expand Up @@ -1896,6 +1902,7 @@ parseStringSegments(SmallVectorImpl<Lexer::StringSegment> &Segments,
ParserStatus S =
parseExprList(tok::l_paren, tok::r_paren,
/*isPostfix=*/false, /*isExprBasic=*/true,
/*allowSepOmission=*/false,
lParen, args, argLabels, argLabelLocs, rParen,
trailingClosureNeverPresent,
SyntaxKind::Unknown);
Expand Down Expand Up @@ -2976,7 +2983,8 @@ Expr *Parser::parseExprAnonClosureArg() {
/// (identifier ':')? expr
///
ParserResult<Expr>
Parser::parseExprList(tok leftTok, tok rightTok, SyntaxKind Kind) {
Parser::parseExprList(tok leftTok, tok rightTok, bool allowSepOmission,
SyntaxKind Kind) {
SmallVector<Expr*, 8> subExprs;
SmallVector<Identifier, 8> subExprNames;
SmallVector<SourceLoc, 8> subExprNameLocs;
Expand All @@ -2985,6 +2993,7 @@ Parser::parseExprList(tok leftTok, tok rightTok, SyntaxKind Kind) {
SourceLoc leftLoc, rightLoc;
ParserStatus status = parseExprList(leftTok, rightTok, /*isPostfix=*/false,
/*isExprBasic=*/true,
allowSepOmission,
leftLoc,
subExprs,
subExprNames,
Expand Down Expand Up @@ -3021,6 +3030,7 @@ Parser::parseExprList(tok leftTok, tok rightTok, SyntaxKind Kind) {
ParserStatus Parser::parseExprList(tok leftTok, tok rightTok,
bool isPostfix,
bool isExprBasic,
bool allowSepOmission,
SourceLoc &leftLoc,
SmallVectorImpl<Expr *> &exprs,
SmallVectorImpl<Identifier> &exprLabels,
Expand All @@ -3039,6 +3049,7 @@ ParserStatus Parser::parseExprList(tok leftTok, tok rightTok,
leftLoc = consumeToken(leftTok);
ParserStatus status = parseList(rightTok, leftLoc, rightLoc,
/*AllowSepAfterLast=*/false,
/*AllowSepOmission=*/allowSepOmission,
rightTok == tok::r_paren
? diag::expected_rparen_expr_list
: diag::expected_rsquare_expr_list,
Expand Down Expand Up @@ -3180,6 +3191,7 @@ Parser::parseExprObjectLiteral(ObjectLiteralExpr::LiteralKind LitKind,

ParserStatus status = parseExprList(tok::l_paren, tok::r_paren,
/*isPostfix=*/true, isExprBasic,
/*allowSepOmission=*/false,
lParenLoc, args, argLabels,
argLabelLocs,
rParenLoc,
Expand Down Expand Up @@ -3223,8 +3235,9 @@ ParserResult<Expr> Parser::parseExprPoundUnknown(SourceLoc LSquareLoc) {
// Parse arguments.
ParserStatus status =
parseExprList(tok::l_paren, tok::r_paren,
/*isPostfix=*/false, /*isExprBasic*/ false, LParenLoc,
args, argLabels, argLabelLocs, RParenLoc, trailingClosure,
/*isPostfix=*/false, /*isExprBasic*/ false,
/*allowSepOmission=*/false, LParenLoc, args, argLabels,
argLabelLocs, RParenLoc, trailingClosure,
SyntaxKind::FunctionCallArgumentList);
if (status.hasCodeCompletion())
return makeParserCodeCompletionResult<Expr>();
Expand Down Expand Up @@ -3337,6 +3350,7 @@ Parser::parseExprCallSuffix(ParserResult<Expr> fn, bool isExprBasic) {

ParserStatus status = parseExprList(tok::l_paren, tok::r_paren,
/*isPostfix=*/true, isExprBasic,
/*allowSepOmission=*/true,
lParenLoc, args, argLabels,
argLabelLocs,
rParenLoc,
Expand Down Expand Up @@ -3451,6 +3465,9 @@ ParserResult<Expr> Parser::parseExprCollection() {
isStartOfDecl() || isStartOfStmt()))
break;

if (Tok.isAtStartOfLine())
continue;

diagnose(Tok, diag::expected_separator, ",")
.fixItInsertAfter(PreviousLoc, ",");
Status.setIsParseError();
Expand Down
3 changes: 3 additions & 0 deletions lib/Parse/ParsePattern.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
bool isClosure = paramContext == ParameterContextKind::Closure;
return parseList(tok::r_paren, leftParenLoc, rightParenLoc,
/*AllowSepAfterLast=*/false,
/*AllowSepOmission=*/false,
diag::expected_rparen_parameter,
SyntaxKind::FunctionParameterList,
[&]() -> ParserStatus {
Expand Down Expand Up @@ -858,6 +859,7 @@ ParserResult<Pattern> Parser::parseTypedPattern() {
ParserStatus status = parseExprList(tok::l_paren, tok::r_paren,
/*isPostfix=*/true,
/*isExprBasic=*/false,
/*allowSepOmission=*/false,
lParenLoc, args, argLabels,
argLabelLocs, rParenLoc,
trailingClosure,
Expand Down Expand Up @@ -1033,6 +1035,7 @@ ParserResult<Pattern> Parser::parsePatternTuple() {
ParserStatus ListStatus =
parseList(tok::r_paren, LPLoc, RPLoc,
/*AllowSepAfterLast=*/false,
/*AllowSepOmission=*/false,
diag::expected_rparen_tuple_pattern_list,
SyntaxKind::TuplePatternElementList,
[&] () -> ParserStatus {
Expand Down
1 change: 1 addition & 0 deletions lib/Parse/ParseStmt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -868,6 +868,7 @@ ParserResult<Stmt> Parser::parseStmtYield(SourceLoc tryLoc) {
status = parseExprList(tok::l_paren, tok::r_paren,
/*postfix (allow trailing closure)*/ false,
/*expr basic (irrelevant)*/ true,
/*allow separator omission*/ false,
lpLoc,
yields, yieldLabels, yieldLabelLocs,
rpLoc,
Expand Down
1 change: 1 addition & 0 deletions lib/Parse/ParseType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,7 @@ ParserResult<TupleTypeRepr> Parser::parseTypeTupleBody() {

ParserStatus Status = parseList(tok::r_paren, LPLoc, RPLoc,
/*AllowSepAfterLast=*/false,
/*AllowSepOmission=*/false,
diag::expected_rparen_tuple_type_list,
SyntaxKind::TupleTypeElementList,
[&] () -> ParserStatus {
Expand Down
24 changes: 18 additions & 6 deletions lib/Parse/Parser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -953,7 +953,8 @@ static SyntaxKind getListElementKind(SyntaxKind ListKind) {

ParserStatus
Parser::parseList(tok RightK, SourceLoc LeftLoc, SourceLoc &RightLoc,
bool AllowSepAfterLast, Diag<> ErrorDiag, SyntaxKind Kind,
bool AllowSepAfterLast, bool AllowSepOmission,
Diag<> ErrorDiag, SyntaxKind Kind,
llvm::function_ref<ParserStatus()> callback) {
auto TokIsStringInterpolationEOF = [&]() -> bool {
return Tok.is(tok::eof) && Tok.getText() == ")" && RightK == tok::r_paren;
Expand Down Expand Up @@ -1014,18 +1015,29 @@ Parser::parseList(tok RightK, SourceLoc LeftLoc, SourceLoc &RightLoc,
}
break;
}
// If we're in a comma-separated list, the next token is at the
// beginning of a new line and can never start an element, break.
if (Tok.isAtStartOfLine() &&
(Tok.is(tok::r_brace) || isStartOfDecl() || isStartOfStmt())) {
break;

if (Tok.isAtStartOfLine()) {
if (AllowSepOmission && Tok.canBeArgumentLabel() &&
peekToken().is(tok::colon))
// If the next token is at the beginning of a new line, could be an
// argument label, is followed by a colon, and separator omission is
// permitted, expect that we'll parse another list element which begins
// with that argument label and continue parsing the list.
continue;
else if (Tok.is(tok::r_brace) || isStartOfDecl() || isStartOfStmt())
// If we're in a comma-separated list, the next token is at the
// beginning of a new line and can never start an element, break.
break;
}
// If we found EOF or such, bailout.
if (Tok.isAny(tok::eof, tok::pound_endif)) {
IsInputIncomplete = true;
break;
}

if (AllowSepOmission && Tok.isAtStartOfLine())
continue;

diagnose(Tok, diag::expected_separator, ",")
.fixItInsertAfter(PreviousLoc, ",");
Status.setIsParseError();
Expand Down
1 change: 1 addition & 0 deletions lib/ParseSIL/ParseSIL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1639,6 +1639,7 @@ bool SILParser::parseSILBBArgsAtBranch(SmallVector<SILValue, 6> &Args,

if (P.parseList(tok::r_paren, LParenLoc, RParenLoc,
/*AllowSepAfterLast=*/false,
/*AllowSepOmission=*/false,
diag::sil_basicblock_arg_rparen,
SyntaxKind::Unknown,
[&]() -> ParserStatus {
Expand Down
Loading