Skip to content

Commit 22652f9

Browse files
rintarorintaro
committed
[Parse] Eliminate backtracking in collection expression parsing
Parsing collection literal expression used to take exponential time depending on the nesting level of the first element. Stop using 'parseList()' because using it complicates libSyntax parsing. rdar://problem/45221238 / https://bugs.swift.org/browse/SR-9220 rdar://problem/38913395 / https://bugs.swift.org/browse/SR-7283
1 parent edc2010 commit 22652f9

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)