Skip to content

Commit 47b987f

Browse files
rintarorintaro
authored
Merge pull request #21487 from rintaro/parse-collection-rdar45221238
[Parse] Eliminate backtracking in collection expression parsing
2 parents a6fb95d + 22652f9 commit 47b987f

File tree

7 files changed

+138
-132
lines changed

7 files changed

+138
-132
lines changed

‎include/swift/Parse/Parser.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1333,8 +1333,7 @@ class Parser {
13331333
ParserResult<Expr> parseExprCallSuffix(ParserResult<Expr> fn,
13341334
bool isExprBasic);
13351335
ParserResult<Expr> parseExprCollection();
1336-
ParserResult<Expr> parseExprArray(SourceLoc LSquareLoc);
1337-
ParserResult<Expr> parseExprDictionary(SourceLoc LSquareLoc);
1336+
ParserResult<Expr> parseExprCollectionElement(Optional<bool> &isDictionary);
13381337
ParserResult<Expr> parseExprPoundAssert();
13391338
ParserResult<Expr> parseExprPoundUnknown(SourceLoc LSquareLoc);
13401339
ParserResult<Expr>

‎lib/Parse/ParseExpr.cpp

Lines changed: 112 additions & 126 deletions
Original file line numberDiff line numberDiff line change
@@ -3343,10 +3343,17 @@ Parser::parseExprCallSuffix(ParserResult<Expr> fn, bool isExprBasic) {
33433343
/// expr-collection:
33443344
/// expr-array
33453345
/// expr-dictionary
3346-
// lsquare-starting ']'
3346+
/// expr-array:
3347+
/// '[' expr (',' expr)* ','? ']'
3348+
/// '[' ']'
3349+
/// expr-dictionary:
3350+
/// '[' expr ':' expr (',' expr ':' expr)* ','? ']'
3351+
/// '[' ':' ']'
33473352
ParserResult<Expr> Parser::parseExprCollection() {
3348-
SyntaxParsingContext ArrayOrDictContext(SyntaxContext);
3353+
SyntaxParsingContext ArrayOrDictContext(SyntaxContext,
3354+
SyntaxContextKind::Expr);
33493355
SourceLoc LSquareLoc = consumeToken(tok::l_square);
3356+
SourceLoc RSquareLoc;
33503357

33513358
Parser::StructureMarkerRAII ParsingCollection(
33523359
*this, LSquareLoc,
@@ -3357,7 +3364,7 @@ ParserResult<Expr> Parser::parseExprCollection() {
33573364
if (SyntaxContext->isEnabled())
33583365
SyntaxContext->addSyntax(
33593366
SyntaxFactory::makeBlankArrayElementList(Context.getSyntaxArena()));
3360-
SourceLoc RSquareLoc = consumeToken(tok::r_square);
3367+
RSquareLoc = consumeToken(tok::r_square);
33613368
ArrayOrDictContext.setCreateSyntax(SyntaxKind::ArrayExpr);
33623369
return makeParserResult(
33633370
ArrayExpr::create(Context, LSquareLoc, {}, {}, RSquareLoc));
@@ -3366,7 +3373,7 @@ ParserResult<Expr> Parser::parseExprCollection() {
33663373
// [:] is always an empty dictionary.
33673374
if (Tok.is(tok::colon) && peekToken().is(tok::r_square)) {
33683375
consumeToken(tok::colon);
3369-
SourceLoc RSquareLoc = consumeToken(tok::r_square);
3376+
RSquareLoc = consumeToken(tok::r_square);
33703377
ArrayOrDictContext.setCreateSyntax(SyntaxKind::DictionaryExpr);
33713378
return makeParserResult(
33723379
DictionaryExpr::create(Context, LSquareLoc, {}, {}, RSquareLoc));
@@ -3381,147 +3388,126 @@ ParserResult<Expr> Parser::parseExprCollection() {
33813388
return parseExprPoundUnknown(LSquareLoc);
33823389
}
33833390

3384-
bool ParseDict;
3391+
ParserStatus Status;
3392+
Optional<bool> isDictionary;
3393+
SmallVector<Expr *, 8> ElementExprs;
3394+
SmallVector<SourceLoc, 8> CommaLocs;
3395+
33853396
{
3386-
BacktrackingScope Scope(*this);
3387-
auto HasDelayedDecl = State->hasDelayedDecl();
3388-
// Parse the first expression.
3389-
ParserResult<Expr> FirstExpr
3390-
= parseExpr(diag::expected_expr_in_collection_literal);
3391-
if (FirstExpr.isNull() || Tok.is(tok::eof)) {
3392-
skipUntil(tok::r_square);
3393-
if (Tok.is(tok::r_square))
3394-
consumeToken();
3395-
Scope.cancelBacktrack();
3396-
ArrayOrDictContext.setCoerceKind(SyntaxContextKind::Expr);
3397-
return FirstExpr;
3398-
}
3399-
if (!HasDelayedDecl)
3400-
State->takeDelayedDeclState();
3401-
// If we have a ':', this is a dictionary literal.
3402-
ParseDict = Tok.is(tok::colon);
3403-
}
3397+
SyntaxParsingContext ListCtx(SyntaxContext, SyntaxContextKind::Expr);
3398+
3399+
while (true) {
3400+
SyntaxParsingContext ElementCtx(SyntaxContext);
3401+
3402+
auto Element = parseExprCollectionElement(isDictionary);
3403+
Status |= Element;
3404+
ElementCtx.setCreateSyntax(*isDictionary ? SyntaxKind::DictionaryElement
3405+
: SyntaxKind::ArrayElement);
3406+
if (Element.isNonNull())
3407+
ElementExprs.push_back(Element.get());
3408+
3409+
// Skip to ']' or ',' in case of error.
3410+
// NOTE: This checks 'Status' instead of 'Element' to silence excessive
3411+
// diagnostics.
3412+
if (Status.isError()) {
3413+
skipUntilDeclRBrace(tok::r_square, tok::comma);
3414+
if (Tok.isNot(tok::comma))
3415+
break;
3416+
}
34043417

3405-
if (ParseDict) {
3406-
ArrayOrDictContext.setCreateSyntax(SyntaxKind::DictionaryExpr);
3407-
return parseExprDictionary(LSquareLoc);
3408-
} else {
3409-
ArrayOrDictContext.setCreateSyntax(SyntaxKind::ArrayExpr);
3410-
return parseExprArray(LSquareLoc);
3411-
}
3412-
}
3418+
// Parse the ',' if exists.
3419+
if (Tok.is(tok::comma)) {
3420+
CommaLocs.push_back(consumeToken());
3421+
if (!Tok.is(tok::r_square))
3422+
continue;
3423+
}
34133424

3414-
/// parseExprArray - Parse an array literal expression.
3415-
///
3416-
/// The lsquare-starting and first expression have already been
3417-
/// parsed, and are passed in as parameters.
3418-
///
3419-
/// expr-array:
3420-
/// '[' expr (',' expr)* ','? ']'
3421-
/// '[' ']'
3422-
ParserResult<Expr> Parser::parseExprArray(SourceLoc LSquareLoc) {
3423-
SmallVector<Expr *, 8> SubExprs;
3424-
SmallVector<SourceLoc, 8> CommaLocs;
3425+
// Close square.
3426+
if (Tok.is(tok::r_square))
3427+
break;
34253428

3426-
SourceLoc RSquareLoc;
3427-
ParserStatus Status;
3428-
bool First = true;
3429-
bool HasError = false;
3430-
Status |= parseList(tok::r_square, LSquareLoc, RSquareLoc,
3431-
/*AllowSepAfterLast=*/true,
3432-
diag::expected_rsquare_array_expr,
3433-
SyntaxKind::ArrayElementList,
3434-
[&] () -> ParserStatus
3435-
{
3436-
ParserResult<Expr> Element
3437-
= parseExpr(diag::expected_expr_in_collection_literal);
3438-
if (Element.isNonNull())
3439-
SubExprs.push_back(Element.get());
3440-
3441-
if (First) {
3442-
if (Tok.isNot(tok::r_square) && Tok.isNot(tok::comma)) {
3443-
diagnose(Tok, diag::expected_separator, ",").
3444-
fixItInsertAfter(PreviousLoc, ",");
3445-
HasError = true;
3429+
// If we found EOF or such, bailout.
3430+
if (Tok.is(tok::eof)) {
3431+
IsInputIncomplete = true;
3432+
break;
34463433
}
3447-
First = false;
3434+
3435+
// If The next token is at the beginning of a new line and can never start
3436+
// an element, break.
3437+
if (Tok.isAtStartOfLine() && (Tok.isAny(tok::r_brace, tok::pound_endif) ||
3438+
isStartOfDecl() || isStartOfStmt()))
3439+
break;
3440+
3441+
diagnose(Tok, diag::expected_separator, ",")
3442+
.fixItInsertAfter(PreviousLoc, ",");
3443+
Status.setIsParseError();
34483444
}
34493445

3450-
if (Tok.is(tok::comma))
3451-
CommaLocs.push_back(Tok.getLoc());
3452-
return Element;
3453-
});
3454-
if (HasError)
3446+
ListCtx.setCreateSyntax(*isDictionary ? SyntaxKind::DictionaryElementList
3447+
: SyntaxKind::ArrayElementList);
3448+
}
3449+
ArrayOrDictContext.setCreateSyntax(*isDictionary ? SyntaxKind::DictionaryExpr
3450+
: SyntaxKind::ArrayExpr);
3451+
3452+
if (Status.isError()) {
3453+
// If we've already got errors, don't emit missing RightK diagnostics.
3454+
RSquareLoc = Tok.is(tok::r_square) ? consumeToken() : PreviousLoc;
3455+
} else if (parseMatchingToken(tok::r_square, RSquareLoc,
3456+
diag::expected_rsquare_array_expr,
3457+
LSquareLoc)) {
34553458
Status.setIsParseError();
3459+
}
34563460

3457-
assert(SubExprs.size() >= 1);
3458-
return makeParserResult(Status,
3459-
ArrayExpr::create(Context, LSquareLoc, SubExprs, CommaLocs,
3460-
RSquareLoc));
3461+
// Don't bother to create expression if any expressions aren't parsed.
3462+
if (ElementExprs.empty())
3463+
return Status;
3464+
3465+
Expr *expr;
3466+
if (*isDictionary)
3467+
expr = DictionaryExpr::create(Context, LSquareLoc, ElementExprs, CommaLocs,
3468+
RSquareLoc);
3469+
else
3470+
expr = ArrayExpr::create(Context, LSquareLoc, ElementExprs, CommaLocs,
3471+
RSquareLoc);
3472+
3473+
return makeParserResult(Status, expr);
34613474
}
34623475

3463-
/// parseExprDictionary - Parse a dictionary literal expression.
3464-
///
3465-
/// The lsquare-starting and first key have already been parsed, and
3466-
/// are passed in as parameters.
3476+
/// parseExprCollectionElement - Parse an element for collection expr.
34673477
///
3468-
/// expr-dictionary:
3469-
/// '[' expr ':' expr (',' expr ':' expr)* ','? ']'
3470-
/// '[' ':' ']'
3471-
ParserResult<Expr> Parser::parseExprDictionary(SourceLoc LSquareLoc) {
3472-
// Each subexpression is a (key, value) tuple.
3473-
// FIXME: We're not tracking the colon locations in the AST.
3474-
SmallVector<Expr *, 8> SubExprs;
3475-
SmallVector<SourceLoc, 8> CommaLocs;
3476-
SourceLoc RSquareLoc;
3477-
3478-
// Function that adds a new key/value pair.
3479-
auto addKeyValuePair = [&](Expr *Key, Expr *Value) -> void {
3480-
Expr *Exprs[] = {Key, Value};
3481-
SubExprs.push_back(TupleExpr::createImplicit(Context, Exprs, { }));
3482-
};
3478+
/// If \p isDictionary is \c None, it's set to \c true if the element is for
3479+
/// dictionary literal, or \c false otherwise.
3480+
ParserResult<Expr>
3481+
Parser::parseExprCollectionElement(Optional<bool> &isDictionary) {
3482+
auto Element = parseExpr(isDictionary.hasValue() && *isDictionary
3483+
? diag::expected_key_in_dictionary_literal
3484+
: diag::expected_expr_in_collection_literal);
34833485

3484-
ParserStatus Status;
3485-
Status |=
3486-
parseList(tok::r_square, LSquareLoc, RSquareLoc,
3487-
/*AllowSepAfterLast=*/true,
3488-
diag::expected_rsquare_array_expr,
3489-
SyntaxKind::DictionaryElementList,
3490-
[&]() -> ParserStatus {
3491-
// Parse the next key.
3492-
ParserResult<Expr> Key;
3493-
3494-
Key = parseExpr(diag::expected_key_in_dictionary_literal);
3495-
if (Key.isNull())
3496-
return Key;
3497-
3498-
// Parse the ':'.
3499-
if (Tok.isNot(tok::colon)) {
3500-
diagnose(Tok, diag::expected_colon_in_dictionary_literal);
3501-
return ParserStatus(Key) | makeParserError();
3502-
}
3503-
consumeToken();
3486+
if (!isDictionary.hasValue())
3487+
isDictionary = Tok.is(tok::colon);
35043488

3505-
// Parse the next value.
3506-
ParserResult<Expr> Value =
3507-
parseExpr(diag::expected_value_in_dictionary_literal);
3489+
if (!*isDictionary)
3490+
return Element;
35083491

3509-
if (Value.isNull())
3510-
Value = makeParserResult(Value, new (Context) ErrorExpr(PreviousLoc));
3492+
if (Element.isNull())
3493+
return Element;
35113494

3512-
// Add this key/value pair.
3513-
addKeyValuePair(Key.get(), Value.get());
3495+
// Parse the ':'.
3496+
if (!consumeIf(tok::colon)) {
3497+
diagnose(Tok, diag::expected_colon_in_dictionary_literal);
3498+
return ParserStatus(Element) | makeParserError();
3499+
}
35143500

3515-
if (Tok.is(tok::comma))
3516-
CommaLocs.push_back(Tok.getLoc());
3501+
// Parse the value.
3502+
auto Value = parseExpr(diag::expected_value_in_dictionary_literal);
35173503

3518-
return ParserStatus(Key) | ParserStatus(Value);
3519-
});
3504+
if (Value.isNull())
3505+
Value = makeParserResult(Value, new (Context) ErrorExpr(PreviousLoc));
35203506

3521-
assert(SubExprs.size() >= 1);
3522-
return makeParserResult(Status, DictionaryExpr::create(Context, LSquareLoc,
3523-
SubExprs, CommaLocs,
3524-
RSquareLoc));
3507+
// Make a tuple of Key Value pair.
3508+
return makeParserResult(
3509+
ParserStatus(Element) | ParserStatus(Value),
3510+
TupleExpr::createImplicit(Context, {Element.get(), Value.get()}, {}));
35253511
}
35263512

35273513
void Parser::addPatternVariablesToScope(ArrayRef<Pattern *> Patterns) {

‎test/Constraints/dictionary_literal.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ func testDefaultExistentials() {
110110
// SR-4952, rdar://problem/32330004 - Assertion failure during swift::ASTVisitor<::FailureDiagnosis,...>::visit
111111
func rdar32330004_1() -> [String: Any] {
112112
return ["a""one": 1, "two": 2, "three": 3] // expected-note {{did you mean to use a dictionary literal instead?}}
113-
// expected-error@-1 2 {{expected ',' separator}}
113+
// expected-error@-1 {{expected ',' separator}}
114114
// expected-error@-2 {{dictionary of type '[String : Any]' cannot be used with array literal}}
115115
}
116116

‎test/Profiler/coverage_closures.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// RUN: %target-swift-frontend -Xllvm -sil-full-demangle -profile-generate -profile-coverage-mapping -emit-sil -module-name coverage_closures %s | %FileCheck %s
22

33
func bar(arr: [(Int32) -> Int32]) {
4-
// CHECK-LABEL: sil_coverage_map {{.*}}// closure #2 (Swift.Int32) -> Swift.Int32 in coverage_closures.bar
4+
// CHECK-LABEL: sil_coverage_map {{.*}}// closure #1 (Swift.Int32) -> Swift.Int32 in coverage_closures.bar
55
// CHECK-NEXT: [[@LINE+1]]:13 -> [[@LINE+1]]:42 : 0
66
for a in [{ (b : Int32) -> Int32 in b }] {
77
a(0)

‎test/SILGen/local_types.swift

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,22 @@
11
// RUN: %target-swift-emit-silgen -parse-as-library -enable-sil-ownership %s -verify | %FileCheck %s
22
// RUN: %target-swift-emit-ir -parse-as-library -enable-sil-ownership %s
33

4-
func function() {
4+
func function1() {
55
return
66

77
class UnreachableClass {} // expected-warning {{code after 'return' will never be executed}}
88
}
99

10+
func function2() {
11+
let _ = [
12+
{
13+
struct S {
14+
var x = 0
15+
}
16+
}
17+
]
18+
}
19+
20+
// CHECK-LABEL: sil private [transparent] [ossa] @$s11local_types9function2yyFyycfU_1SL_V1xSivpfi : $@convention(thin) () -> Int
21+
1022
// CHECK-LABEL: sil_vtable UnreachableClass

‎test/Syntax/diagnostics_verify.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ if true {
99
if false {
1010
[.]
1111
// expected-error@-1 {{expected identifier after '.' expression}}
12-
// expected-error@-2 2 {{unknown expression syntax exists in the source}}
12+
// expected-error@-2 {{unknown expression syntax exists in the source}}
1313
}
1414

1515
class { // expected-error {{unknown declaration syntax exists in the source}}

‎validation-test/compiler_scale/parse_array_nested.swift.gyb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// RUN: %scale-test -parse --begin 0 --end 10 --step 1 --select NumASTBytes %s
2+
3+
%for i in range(1, N + 1):
4+
[
5+
%end
6+
1
7+
%for i in range(1, N + 1):
8+
]
9+
%end

0 commit comments

Comments
 (0)