From 43c13ca6b049f3e433d0b28c159aeecfe7d88890 Mon Sep 17 00:00:00 2001 From: Joe Groff Date: Wed, 7 Feb 2024 18:49:11 -0800 Subject: [PATCH] Parse `_borrowing x` contextually as a pattern binding when `.borrowingSwitch` feature is enabled. The existing parsing under `.referenceBindings` parses this (along with `_mutating` and `_consuming`) as unconditional keywords, which would be a source break we can't ship. Narrow the behavior so that `_borrowing` is only parsed immediately before an identifier in a pattern. --- .../SyntaxSupport/ExperimentalFeatures.swift | 3 + .../Sources/SyntaxSupport/KeywordSpec.swift | 23 +++- .../swiftparser/ParserTokenSpecSetFile.swift | 19 ++- Sources/SwiftParser/Expressions.swift | 18 ++- Sources/SwiftParser/Patterns.swift | 57 +++++--- .../generated/ExperimentalFeatures.swift | 3 + .../generated/Parser+TokenSpecSet.swift | 6 +- .../translated/MatchingPatternsTests.swift | 128 ++++++++++++++++++ 8 files changed, 225 insertions(+), 32 deletions(-) diff --git a/CodeGeneration/Sources/SyntaxSupport/ExperimentalFeatures.swift b/CodeGeneration/Sources/SyntaxSupport/ExperimentalFeatures.swift index c4009899f63..c1ad5793fed 100644 --- a/CodeGeneration/Sources/SyntaxSupport/ExperimentalFeatures.swift +++ b/CodeGeneration/Sources/SyntaxSupport/ExperimentalFeatures.swift @@ -18,6 +18,7 @@ public enum ExperimentalFeature: String, CaseIterable { case doExpressions case nonescapableTypes case transferringArgsAndResults + case borrowingSwitch /// The name of the feature, which is used in the doc comment. public var featureName: String { @@ -32,6 +33,8 @@ public enum ExperimentalFeature: String, CaseIterable { return "NonEscableTypes" case .transferringArgsAndResults: return "TransferringArgsAndResults" + case .borrowingSwitch: + return "borrowing pattern matching" } } diff --git a/CodeGeneration/Sources/SyntaxSupport/KeywordSpec.swift b/CodeGeneration/Sources/SyntaxSupport/KeywordSpec.swift index 75823237eef..76588066001 100644 --- a/CodeGeneration/Sources/SyntaxSupport/KeywordSpec.swift +++ b/CodeGeneration/Sources/SyntaxSupport/KeywordSpec.swift @@ -19,6 +19,7 @@ public struct KeywordSpec { /// The experimental feature the keyword is part of, or `nil` if this isn't /// for an experimental feature. public let experimentalFeature: ExperimentalFeature? + public let experimentalFeature2: ExperimentalFeature? /// Indicates if the token kind is switched from being an identifier to a keyword in the lexer. public let isLexerClassified: Bool @@ -59,6 +60,26 @@ public struct KeywordSpec { ) { self.name = name self.experimentalFeature = experimentalFeature + self.experimentalFeature2 = nil + self.isLexerClassified = isLexerClassified + } + + /// Initializes a new `KeywordSpec` instance. + /// + /// - Parameters: + /// - name: A name of the keyword. + /// - experimentalFeature: The experimental feature the keyword is part of, or `nil` if this isn't for an experimental feature. + /// - or: A second experimental feature the keyword is also part of, or `nil` if this isn't for an experimental feature. + /// - isLexerClassified: Indicates if the token kind is switched from being an identifier to a keyword in the lexer. + init( + _ name: String, + experimentalFeature: ExperimentalFeature, + or experimentalFeature2: ExperimentalFeature, + isLexerClassified: Bool = false + ) { + self.name = name + self.experimentalFeature = experimentalFeature + self.experimentalFeature2 = experimentalFeature2 self.isLexerClassified = isLexerClassified } } @@ -720,7 +741,7 @@ public enum Keyword: CaseIterable { case .yield: return KeywordSpec("yield") case ._borrowing: - return KeywordSpec("_borrowing", experimentalFeature: .referenceBindings) + return KeywordSpec("_borrowing", experimentalFeature: .referenceBindings, or: .borrowingSwitch) case ._consuming: return KeywordSpec("_consuming", experimentalFeature: .referenceBindings) case ._mutating: diff --git a/CodeGeneration/Sources/generate-swift-syntax/templates/swiftparser/ParserTokenSpecSetFile.swift b/CodeGeneration/Sources/generate-swift-syntax/templates/swiftparser/ParserTokenSpecSetFile.swift index 4e162beaad3..6a48eeff206 100644 --- a/CodeGeneration/Sources/generate-swift-syntax/templates/swiftparser/ParserTokenSpecSetFile.swift +++ b/CodeGeneration/Sources/generate-swift-syntax/templates/swiftparser/ParserTokenSpecSetFile.swift @@ -15,11 +15,14 @@ import SwiftSyntaxBuilder import SyntaxSupport import Utils -func tokenCaseMatch(_ caseName: TokenSyntax, experimentalFeature: ExperimentalFeature?) -> SwitchCaseSyntax { - let whereClause = - experimentalFeature.map { - "where experimentalFeatures.contains(.\($0.token))" - } ?? "" +func tokenCaseMatch(_ caseName: TokenSyntax, experimentalFeature: ExperimentalFeature?, experimentalFeature2: ExperimentalFeature?) -> SwitchCaseSyntax { + var whereClause = "" + if let feature = experimentalFeature { + whereClause += "where experimentalFeatures.contains(.\(feature.token))" + if let feature2 = experimentalFeature2 { + whereClause += " || experimentalFeatures.contains(.\(feature2.token))" + } + } return "case TokenSpec(.\(caseName))\(raw: whereClause): self = .\(caseName)" } @@ -57,12 +60,14 @@ let parserTokenSpecSetFile = SourceFileSyntax(leadingTrivia: copyrightHeader) { case .keyword(let keyword): tokenCaseMatch( keyword.spec.varOrCaseName, - experimentalFeature: keyword.spec.experimentalFeature + experimentalFeature: keyword.spec.experimentalFeature, + experimentalFeature2: keyword.spec.experimentalFeature2 ) case .token(let token): tokenCaseMatch( token.spec.varOrCaseName, - experimentalFeature: token.spec.experimentalFeature + experimentalFeature: token.spec.experimentalFeature, + experimentalFeature2: nil ) } } diff --git a/Sources/SwiftParser/Expressions.swift b/Sources/SwiftParser/Expressions.swift index ce8f0917e67..f9f160e1135 100644 --- a/Sources/SwiftParser/Expressions.swift +++ b/Sources/SwiftParser/Expressions.swift @@ -118,9 +118,21 @@ extension Parser { // // Only do this if we're parsing a pattern, to improve QoI on malformed // expressions followed by (e.g.) let/var decls. - if pattern != .none, self.at(anyIn: MatchingPatternStart.self) != nil { - let pattern = self.parseMatchingPattern(context: .matching) - return RawExprSyntax(RawPatternExprSyntax(pattern: pattern, arena: self.arena)) + if pattern != .none { + switch self.at(anyIn: MatchingPatternStart.self) { + case (spec: .rhs(let bindingIntroducer), handle: _)? where self.withLookahead { $0.shouldParsePatternBinding(introducer: bindingIntroducer) }: + fallthrough + case (spec: .lhs(_), handle: _)?: + let pattern = self.parseMatchingPattern(context: .matching) + return RawExprSyntax(RawPatternExprSyntax(pattern: pattern, arena: self.arena)) + + // If the token can't start a pattern, or contextually isn't parsed as a + // binding introducer, handle as the start of a normal expression. + case (spec: .rhs(_), handle: _)?, + nil: + // Not a pattern. Break out to parse as a normal expression below. + break + } } return RawExprSyntax(self.parseSequenceExpression(flavor: flavor, pattern: pattern)) } diff --git a/Sources/SwiftParser/Patterns.swift b/Sources/SwiftParser/Patterns.swift index 9da0f4e0ac5..207db532e9b 100644 --- a/Sources/SwiftParser/Patterns.swift +++ b/Sources/SwiftParser/Patterns.swift @@ -67,7 +67,19 @@ extension Parser { arena: self.arena ) ) - case (.lhs(.identifier), let handle)?: + case (.rhs(let introducer), let handle)? where self.withLookahead { $0.shouldParsePatternBinding(introducer: introducer) }: + let bindingSpecifier = self.eat(handle) + let value = self.parsePattern() + return RawPatternSyntax( + RawValueBindingPatternSyntax( + bindingSpecifier: bindingSpecifier, + pattern: value, + arena: self.arena + ) + ) + case (.lhs(.identifier), let handle)?, + // If we shouldn't contextually parse a pattern binding introducer (because the previous pattern match guard failed), then parse it as an identifier. + (.rhs(_), let handle)?: let identifier = self.eat(handle) return RawPatternSyntax( RawIdentifierPatternSyntax( @@ -85,16 +97,6 @@ extension Parser { arena: self.arena ) ) - case (.rhs, let handle)?: - let bindingSpecifier = self.eat(handle) - let value = self.parsePattern() - return RawPatternSyntax( - RawValueBindingPatternSyntax( - bindingSpecifier: bindingSpecifier, - pattern: value, - arena: self.arena - ) - ) case nil: break } @@ -218,7 +220,7 @@ extension Parser { arena: self.arena ) ) - case (.rhs, let handle)?: + case (.rhs(let introducer), let handle)? where self.withLookahead { $0.shouldParsePatternBinding(introducer: introducer) }: let bindingSpecifier = self.eat(handle) let value = self.parseMatchingPattern(context: .bindingIntroducer) return RawPatternSyntax( @@ -228,7 +230,8 @@ extension Parser { arena: self.arena ) ) - case nil: + case (.rhs(_), _)?, + nil: break } @@ -253,6 +256,21 @@ extension Parser { // MARK: Lookahead extension Parser.Lookahead { + /// Returns true if we should parse a pattern binding specifier contextually + /// as one. + mutating func shouldParsePatternBinding(introducer: ValueBindingPatternSyntax.BindingSpecifierOptions) -> Bool { + switch introducer { + // TODO: the other ownership modifiers (borrowing/consuming/mutating) more + // than likely need to be made contextual as well before finalizing their + // grammar. + case ._borrowing where experimentalFeatures.contains(.borrowingSwitch): + return peek(isAt: TokenSpec(.identifier, allowAtStartOfLine: false)) + default: + // Other keywords can be parsed unconditionally. + return true + } + } + /// pattern ::= identifier /// pattern ::= '_' /// pattern ::= pattern-tuple @@ -288,15 +306,18 @@ extension Parser.Lookahead { > switch self.at(anyIn: PatternStartTokens.self) { - case (.lhs(.identifier), let handle)?, - (.lhs(.wildcard), let handle)?: - self.eat(handle) - return true case (.lhs(.leftParen), _)?: return self.canParsePatternTuple() - case (.rhs, let handle)?: + case (.rhs(let introducer), let handle)? where shouldParsePatternBinding(introducer: introducer): + // Parse as a binding introducer, like `let x`. self.eat(handle) return self.canParsePattern() + case (.lhs(.identifier), let handle)?, + (.lhs(.wildcard), let handle)?, + // If a binding introducer is not contextually introducing a binding, then parse like an identifier. + (.rhs(_), let handle)?: + self.eat(handle) + return true case nil: return false } diff --git a/Sources/SwiftParser/generated/ExperimentalFeatures.swift b/Sources/SwiftParser/generated/ExperimentalFeatures.swift index f8bf18e5b79..6fae76a97a2 100644 --- a/Sources/SwiftParser/generated/ExperimentalFeatures.swift +++ b/Sources/SwiftParser/generated/ExperimentalFeatures.swift @@ -38,4 +38,7 @@ extension Parser.ExperimentalFeatures { /// Whether to enable the parsing of TransferringArgsAndResults. public static let transferringArgsAndResults = Self (rawValue: 1 << 4) + + /// Whether to enable the parsing of borrowing pattern matching. + public static let borrowingSwitch = Self (rawValue: 1 << 5) } diff --git a/Sources/SwiftParser/generated/Parser+TokenSpecSet.swift b/Sources/SwiftParser/generated/Parser+TokenSpecSet.swift index 26288849e7a..05e87210801 100644 --- a/Sources/SwiftParser/generated/Parser+TokenSpecSet.swift +++ b/Sources/SwiftParser/generated/Parser+TokenSpecSet.swift @@ -2318,7 +2318,7 @@ extension OptionalBindingConditionSyntax { self = .inout case TokenSpec(._mutating) where experimentalFeatures.contains(.referenceBindings): self = ._mutating - case TokenSpec(._borrowing) where experimentalFeatures.contains(.referenceBindings): + case TokenSpec(._borrowing) where experimentalFeatures.contains(.referenceBindings) || experimentalFeatures.contains(.borrowingSwitch): self = ._borrowing case TokenSpec(._consuming) where experimentalFeatures.contains(.referenceBindings): self = ._consuming @@ -2992,7 +2992,7 @@ extension ValueBindingPatternSyntax { self = .inout case TokenSpec(._mutating) where experimentalFeatures.contains(.referenceBindings): self = ._mutating - case TokenSpec(._borrowing) where experimentalFeatures.contains(.referenceBindings): + case TokenSpec(._borrowing) where experimentalFeatures.contains(.referenceBindings) || experimentalFeatures.contains(.borrowingSwitch): self = ._borrowing case TokenSpec(._consuming) where experimentalFeatures.contains(.referenceBindings): self = ._consuming @@ -3064,7 +3064,7 @@ extension VariableDeclSyntax { self = .inout case TokenSpec(._mutating) where experimentalFeatures.contains(.referenceBindings): self = ._mutating - case TokenSpec(._borrowing) where experimentalFeatures.contains(.referenceBindings): + case TokenSpec(._borrowing) where experimentalFeatures.contains(.referenceBindings) || experimentalFeatures.contains(.borrowingSwitch): self = ._borrowing case TokenSpec(._consuming) where experimentalFeatures.contains(.referenceBindings): self = ._consuming diff --git a/Tests/SwiftParserTest/translated/MatchingPatternsTests.swift b/Tests/SwiftParserTest/translated/MatchingPatternsTests.swift index 697652356b5..4036caf2821 100644 --- a/Tests/SwiftParserTest/translated/MatchingPatternsTests.swift +++ b/Tests/SwiftParserTest/translated/MatchingPatternsTests.swift @@ -13,6 +13,7 @@ // This test file has been translated from swift/test/Parse/matching_patterns.swift @_spi(ExperimentalLanguageFeatures) import SwiftParser +@_spi(ExperimentalLanguageFeatures) @_spi(RawSyntax) import SwiftSyntax import XCTest final class MatchingPatternsTests: ParserTestCase { @@ -606,4 +607,131 @@ final class MatchingPatternsTests: ParserTestCase { experimentalFeatures: .referenceBindings ) } + + func testBorrowingContextualParsing() { + assertParse( + """ + switch 42 { + case _borrowing .foo(): // parses as `_borrowing.foo()` as before + break + } + """, + substructure: ExprSyntax(DeclReferenceExprSyntax(baseName: .identifier("_borrowing"))), + experimentalFeatures: .borrowingSwitch + ) + + assertParse( + """ + switch 42 { + case _borrowing (): // parses as `_borrowing()` as before + break + } + """, + substructure: ExprSyntax(DeclReferenceExprSyntax(baseName: .identifier("_borrowing"))), + experimentalFeatures: .borrowingSwitch + ) + + assertParse( + """ + switch 42 { + case _borrowing x: // parses as binding + break + } + """, + substructure: PatternSyntax( + ValueBindingPatternSyntax( + bindingSpecifier: .keyword(._borrowing), + pattern: PatternSyntax(IdentifierPatternSyntax(identifier: .identifier("x"))) + ) + ), + experimentalFeatures: .borrowingSwitch + ) + + assertParse( + """ + switch bar { + case .payload(_borrowing x): // parses as binding + break + } + """, + substructure: PatternSyntax( + ValueBindingPatternSyntax( + bindingSpecifier: .keyword(._borrowing), + pattern: PatternSyntax(IdentifierPatternSyntax(identifier: .identifier("x"))) + ) + ), + experimentalFeatures: .borrowingSwitch + ) + + assertParse( + """ + switch bar { + case _borrowing x.member: // parses as var introducer surrounding postfix expression (which never is valid) + break + } + """, + substructure: PatternSyntax( + ValueBindingPatternSyntax( + bindingSpecifier: .keyword(._borrowing), + pattern: ExpressionPatternSyntax( + expression: MemberAccessExprSyntax( + base: DeclReferenceExprSyntax(baseName: .identifier("x")), + declName: DeclReferenceExprSyntax(baseName: .identifier("member")) + ) + ) + ) + ), + experimentalFeatures: .borrowingSwitch + ) + assertParse( + """ + switch 42 { + case let _borrowing: // parses as let binding named '_borrowing' + break + } + """, + substructure: PatternSyntax( + ValueBindingPatternSyntax( + bindingSpecifier: .keyword(.let), + pattern: PatternSyntax(IdentifierPatternSyntax(identifier: .identifier("_borrowing"))) + ) + ), + experimentalFeatures: .borrowingSwitch + ) + assertParse( + """ + switch 42 { + case _borrowing + _borrowing: // parses as expr pattern + break + } + """, + substructure: ExprSyntax(DeclReferenceExprSyntax(baseName: .identifier("_borrowing"))), + experimentalFeatures: .borrowingSwitch + ) + assertParse( + """ + switch 42 { + case _borrowing(let _borrowing): // parses as let binding named '_borrowing' inside a case pattern named 'borrowing' + break + } + """, + substructure: PatternSyntax( + ValueBindingPatternSyntax( + bindingSpecifier: .keyword(.let), + pattern: PatternSyntax(IdentifierPatternSyntax(identifier: .identifier("_borrowing"))) + ) + ), + experimentalFeatures: .borrowingSwitch + ) + assertParse( + """ + switch 42 { + case {}(_borrowing + _borrowing): // parses as expr pattern + break + } + """, + substructure: ExprSyntax(DeclReferenceExprSyntax(baseName: .identifier("_borrowing"))), + experimentalFeatures: .borrowingSwitch + ) + } }