From 22652f9e88bed20813d461e8aade3dd383ff858e Mon Sep 17 00:00:00 2001 From: Rintaro Ishizaki Date: Fri, 21 Dec 2018 15:28:42 +0900 Subject: [PATCH] [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 --- include/swift/Parse/Parser.h | 3 +- lib/Parse/ParseExpr.cpp | 238 +++++++++--------- test/Constraints/dictionary_literal.swift | 2 +- test/Profiler/coverage_closures.swift | 2 +- test/SILGen/local_types.swift | 14 +- test/Syntax/diagnostics_verify.swift | 2 +- .../parse_array_nested.swift.gyb | 9 + 7 files changed, 138 insertions(+), 132 deletions(-) create mode 100644 validation-test/compiler_scale/parse_array_nested.swift.gyb diff --git a/include/swift/Parse/Parser.h b/include/swift/Parse/Parser.h index 7a25f527ad637..c6a1e76b96675 100644 --- a/include/swift/Parse/Parser.h +++ b/include/swift/Parse/Parser.h @@ -1333,8 +1333,7 @@ class Parser { ParserResult parseExprCallSuffix(ParserResult fn, bool isExprBasic); ParserResult parseExprCollection(); - ParserResult parseExprArray(SourceLoc LSquareLoc); - ParserResult parseExprDictionary(SourceLoc LSquareLoc); + ParserResult parseExprCollectionElement(Optional &isDictionary); ParserResult parseExprPoundAssert(); ParserResult parseExprPoundUnknown(SourceLoc LSquareLoc); ParserResult diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp index 79499564f9ad8..6ddd812a5a170 100644 --- a/lib/Parse/ParseExpr.cpp +++ b/lib/Parse/ParseExpr.cpp @@ -3343,10 +3343,17 @@ Parser::parseExprCallSuffix(ParserResult fn, bool isExprBasic) { /// expr-collection: /// expr-array /// expr-dictionary -// lsquare-starting ']' +/// expr-array: +/// '[' expr (',' expr)* ','? ']' +/// '[' ']' +/// expr-dictionary: +/// '[' expr ':' expr (',' expr ':' expr)* ','? ']' +/// '[' ':' ']' ParserResult Parser::parseExprCollection() { - SyntaxParsingContext ArrayOrDictContext(SyntaxContext); + SyntaxParsingContext ArrayOrDictContext(SyntaxContext, + SyntaxContextKind::Expr); SourceLoc LSquareLoc = consumeToken(tok::l_square); + SourceLoc RSquareLoc; Parser::StructureMarkerRAII ParsingCollection( *this, LSquareLoc, @@ -3357,7 +3364,7 @@ ParserResult Parser::parseExprCollection() { if (SyntaxContext->isEnabled()) SyntaxContext->addSyntax( SyntaxFactory::makeBlankArrayElementList(Context.getSyntaxArena())); - SourceLoc RSquareLoc = consumeToken(tok::r_square); + RSquareLoc = consumeToken(tok::r_square); ArrayOrDictContext.setCreateSyntax(SyntaxKind::ArrayExpr); return makeParserResult( ArrayExpr::create(Context, LSquareLoc, {}, {}, RSquareLoc)); @@ -3366,7 +3373,7 @@ ParserResult Parser::parseExprCollection() { // [:] is always an empty dictionary. if (Tok.is(tok::colon) && peekToken().is(tok::r_square)) { consumeToken(tok::colon); - SourceLoc RSquareLoc = consumeToken(tok::r_square); + RSquareLoc = consumeToken(tok::r_square); ArrayOrDictContext.setCreateSyntax(SyntaxKind::DictionaryExpr); return makeParserResult( DictionaryExpr::create(Context, LSquareLoc, {}, {}, RSquareLoc)); @@ -3381,147 +3388,126 @@ ParserResult Parser::parseExprCollection() { return parseExprPoundUnknown(LSquareLoc); } - bool ParseDict; + ParserStatus Status; + Optional isDictionary; + SmallVector ElementExprs; + SmallVector CommaLocs; + { - BacktrackingScope Scope(*this); - auto HasDelayedDecl = State->hasDelayedDecl(); - // Parse the first expression. - ParserResult FirstExpr - = parseExpr(diag::expected_expr_in_collection_literal); - if (FirstExpr.isNull() || Tok.is(tok::eof)) { - skipUntil(tok::r_square); - if (Tok.is(tok::r_square)) - consumeToken(); - Scope.cancelBacktrack(); - ArrayOrDictContext.setCoerceKind(SyntaxContextKind::Expr); - return FirstExpr; - } - if (!HasDelayedDecl) - State->takeDelayedDeclState(); - // If we have a ':', this is a dictionary literal. - ParseDict = Tok.is(tok::colon); - } + SyntaxParsingContext ListCtx(SyntaxContext, SyntaxContextKind::Expr); + + while (true) { + SyntaxParsingContext ElementCtx(SyntaxContext); + + auto Element = parseExprCollectionElement(isDictionary); + Status |= Element; + ElementCtx.setCreateSyntax(*isDictionary ? SyntaxKind::DictionaryElement + : SyntaxKind::ArrayElement); + if (Element.isNonNull()) + ElementExprs.push_back(Element.get()); + + // Skip to ']' or ',' in case of error. + // NOTE: This checks 'Status' instead of 'Element' to silence excessive + // diagnostics. + if (Status.isError()) { + skipUntilDeclRBrace(tok::r_square, tok::comma); + if (Tok.isNot(tok::comma)) + break; + } - if (ParseDict) { - ArrayOrDictContext.setCreateSyntax(SyntaxKind::DictionaryExpr); - return parseExprDictionary(LSquareLoc); - } else { - ArrayOrDictContext.setCreateSyntax(SyntaxKind::ArrayExpr); - return parseExprArray(LSquareLoc); - } -} + // Parse the ',' if exists. + if (Tok.is(tok::comma)) { + CommaLocs.push_back(consumeToken()); + if (!Tok.is(tok::r_square)) + continue; + } -/// parseExprArray - Parse an array literal expression. -/// -/// The lsquare-starting and first expression have already been -/// parsed, and are passed in as parameters. -/// -/// expr-array: -/// '[' expr (',' expr)* ','? ']' -/// '[' ']' -ParserResult Parser::parseExprArray(SourceLoc LSquareLoc) { - SmallVector SubExprs; - SmallVector CommaLocs; + // Close square. + if (Tok.is(tok::r_square)) + break; - SourceLoc RSquareLoc; - ParserStatus Status; - bool First = true; - bool HasError = false; - Status |= parseList(tok::r_square, LSquareLoc, RSquareLoc, - /*AllowSepAfterLast=*/true, - diag::expected_rsquare_array_expr, - SyntaxKind::ArrayElementList, - [&] () -> ParserStatus - { - ParserResult Element - = parseExpr(diag::expected_expr_in_collection_literal); - if (Element.isNonNull()) - SubExprs.push_back(Element.get()); - - if (First) { - if (Tok.isNot(tok::r_square) && Tok.isNot(tok::comma)) { - diagnose(Tok, diag::expected_separator, ","). - fixItInsertAfter(PreviousLoc, ","); - HasError = true; + // If we found EOF or such, bailout. + if (Tok.is(tok::eof)) { + IsInputIncomplete = true; + break; } - First = false; + + // If The next token is at the beginning of a new line and can never start + // an element, break. + if (Tok.isAtStartOfLine() && (Tok.isAny(tok::r_brace, tok::pound_endif) || + isStartOfDecl() || isStartOfStmt())) + break; + + diagnose(Tok, diag::expected_separator, ",") + .fixItInsertAfter(PreviousLoc, ","); + Status.setIsParseError(); } - if (Tok.is(tok::comma)) - CommaLocs.push_back(Tok.getLoc()); - return Element; - }); - if (HasError) + ListCtx.setCreateSyntax(*isDictionary ? SyntaxKind::DictionaryElementList + : SyntaxKind::ArrayElementList); + } + ArrayOrDictContext.setCreateSyntax(*isDictionary ? SyntaxKind::DictionaryExpr + : SyntaxKind::ArrayExpr); + + if (Status.isError()) { + // If we've already got errors, don't emit missing RightK diagnostics. + RSquareLoc = Tok.is(tok::r_square) ? consumeToken() : PreviousLoc; + } else if (parseMatchingToken(tok::r_square, RSquareLoc, + diag::expected_rsquare_array_expr, + LSquareLoc)) { Status.setIsParseError(); + } - assert(SubExprs.size() >= 1); - return makeParserResult(Status, - ArrayExpr::create(Context, LSquareLoc, SubExprs, CommaLocs, - RSquareLoc)); + // Don't bother to create expression if any expressions aren't parsed. + if (ElementExprs.empty()) + return Status; + + Expr *expr; + if (*isDictionary) + expr = DictionaryExpr::create(Context, LSquareLoc, ElementExprs, CommaLocs, + RSquareLoc); + else + expr = ArrayExpr::create(Context, LSquareLoc, ElementExprs, CommaLocs, + RSquareLoc); + + return makeParserResult(Status, expr); } -/// parseExprDictionary - Parse a dictionary literal expression. -/// -/// The lsquare-starting and first key have already been parsed, and -/// are passed in as parameters. +/// parseExprCollectionElement - Parse an element for collection expr. /// -/// expr-dictionary: -/// '[' expr ':' expr (',' expr ':' expr)* ','? ']' -/// '[' ':' ']' -ParserResult Parser::parseExprDictionary(SourceLoc LSquareLoc) { - // Each subexpression is a (key, value) tuple. - // FIXME: We're not tracking the colon locations in the AST. - SmallVector SubExprs; - SmallVector CommaLocs; - SourceLoc RSquareLoc; - - // Function that adds a new key/value pair. - auto addKeyValuePair = [&](Expr *Key, Expr *Value) -> void { - Expr *Exprs[] = {Key, Value}; - SubExprs.push_back(TupleExpr::createImplicit(Context, Exprs, { })); - }; +/// If \p isDictionary is \c None, it's set to \c true if the element is for +/// dictionary literal, or \c false otherwise. +ParserResult +Parser::parseExprCollectionElement(Optional &isDictionary) { + auto Element = parseExpr(isDictionary.hasValue() && *isDictionary + ? diag::expected_key_in_dictionary_literal + : diag::expected_expr_in_collection_literal); - ParserStatus Status; - Status |= - parseList(tok::r_square, LSquareLoc, RSquareLoc, - /*AllowSepAfterLast=*/true, - diag::expected_rsquare_array_expr, - SyntaxKind::DictionaryElementList, - [&]() -> ParserStatus { - // Parse the next key. - ParserResult Key; - - Key = parseExpr(diag::expected_key_in_dictionary_literal); - if (Key.isNull()) - return Key; - - // Parse the ':'. - if (Tok.isNot(tok::colon)) { - diagnose(Tok, diag::expected_colon_in_dictionary_literal); - return ParserStatus(Key) | makeParserError(); - } - consumeToken(); + if (!isDictionary.hasValue()) + isDictionary = Tok.is(tok::colon); - // Parse the next value. - ParserResult Value = - parseExpr(diag::expected_value_in_dictionary_literal); + if (!*isDictionary) + return Element; - if (Value.isNull()) - Value = makeParserResult(Value, new (Context) ErrorExpr(PreviousLoc)); + if (Element.isNull()) + return Element; - // Add this key/value pair. - addKeyValuePair(Key.get(), Value.get()); + // Parse the ':'. + if (!consumeIf(tok::colon)) { + diagnose(Tok, diag::expected_colon_in_dictionary_literal); + return ParserStatus(Element) | makeParserError(); + } - if (Tok.is(tok::comma)) - CommaLocs.push_back(Tok.getLoc()); + // Parse the value. + auto Value = parseExpr(diag::expected_value_in_dictionary_literal); - return ParserStatus(Key) | ParserStatus(Value); - }); + if (Value.isNull()) + Value = makeParserResult(Value, new (Context) ErrorExpr(PreviousLoc)); - assert(SubExprs.size() >= 1); - return makeParserResult(Status, DictionaryExpr::create(Context, LSquareLoc, - SubExprs, CommaLocs, - RSquareLoc)); + // Make a tuple of Key Value pair. + return makeParserResult( + ParserStatus(Element) | ParserStatus(Value), + TupleExpr::createImplicit(Context, {Element.get(), Value.get()}, {})); } void Parser::addPatternVariablesToScope(ArrayRef Patterns) { diff --git a/test/Constraints/dictionary_literal.swift b/test/Constraints/dictionary_literal.swift index afdd205e3dc91..e60592b3a4169 100644 --- a/test/Constraints/dictionary_literal.swift +++ b/test/Constraints/dictionary_literal.swift @@ -110,7 +110,7 @@ func testDefaultExistentials() { // SR-4952, rdar://problem/32330004 - Assertion failure during swift::ASTVisitor<::FailureDiagnosis,...>::visit func rdar32330004_1() -> [String: Any] { return ["a""one": 1, "two": 2, "three": 3] // expected-note {{did you mean to use a dictionary literal instead?}} - // expected-error@-1 2 {{expected ',' separator}} + // expected-error@-1 {{expected ',' separator}} // expected-error@-2 {{dictionary of type '[String : Any]' cannot be used with array literal}} } diff --git a/test/Profiler/coverage_closures.swift b/test/Profiler/coverage_closures.swift index c87e94032d6da..4f7b893a244eb 100644 --- a/test/Profiler/coverage_closures.swift +++ b/test/Profiler/coverage_closures.swift @@ -1,7 +1,7 @@ // RUN: %target-swift-frontend -Xllvm -sil-full-demangle -profile-generate -profile-coverage-mapping -emit-sil -module-name coverage_closures %s | %FileCheck %s func bar(arr: [(Int32) -> Int32]) { -// CHECK-LABEL: sil_coverage_map {{.*}}// closure #2 (Swift.Int32) -> Swift.Int32 in coverage_closures.bar +// CHECK-LABEL: sil_coverage_map {{.*}}// closure #1 (Swift.Int32) -> Swift.Int32 in coverage_closures.bar // CHECK-NEXT: [[@LINE+1]]:13 -> [[@LINE+1]]:42 : 0 for a in [{ (b : Int32) -> Int32 in b }] { a(0) diff --git a/test/SILGen/local_types.swift b/test/SILGen/local_types.swift index bc22189f3abf4..0cecdd1a08482 100644 --- a/test/SILGen/local_types.swift +++ b/test/SILGen/local_types.swift @@ -1,10 +1,22 @@ // RUN: %target-swift-emit-silgen -parse-as-library -enable-sil-ownership %s -verify | %FileCheck %s // RUN: %target-swift-emit-ir -parse-as-library -enable-sil-ownership %s -func function() { +func function1() { return class UnreachableClass {} // expected-warning {{code after 'return' will never be executed}} } +func function2() { + let _ = [ + { + struct S { + var x = 0 + } + } + ] +} + +// CHECK-LABEL: sil private [transparent] [ossa] @$s11local_types9function2yyFyycfU_1SL_V1xSivpfi : $@convention(thin) () -> Int + // CHECK-LABEL: sil_vtable UnreachableClass diff --git a/test/Syntax/diagnostics_verify.swift b/test/Syntax/diagnostics_verify.swift index b0b3c7eae3cea..44815b13efed5 100644 --- a/test/Syntax/diagnostics_verify.swift +++ b/test/Syntax/diagnostics_verify.swift @@ -9,7 +9,7 @@ if true { if false { [.] // expected-error@-1 {{expected identifier after '.' expression}} - // expected-error@-2 2 {{unknown expression syntax exists in the source}} + // expected-error@-2 {{unknown expression syntax exists in the source}} } class { // expected-error {{unknown declaration syntax exists in the source}} diff --git a/validation-test/compiler_scale/parse_array_nested.swift.gyb b/validation-test/compiler_scale/parse_array_nested.swift.gyb new file mode 100644 index 0000000000000..e75e79474f1e7 --- /dev/null +++ b/validation-test/compiler_scale/parse_array_nested.swift.gyb @@ -0,0 +1,9 @@ +// RUN: %scale-test -parse --begin 0 --end 10 --step 1 --select NumASTBytes %s + + %for i in range(1, N + 1): + [ + %end + 1 + %for i in range(1, N + 1): + ] + %end