Skip to content

Parse _borrowing x contextually as a pattern binding when .borrowingSwitch feature is enabled. #2487

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.

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -32,6 +33,8 @@ public enum ExperimentalFeature: String, CaseIterable {
return "NonEscableTypes"
case .transferringArgsAndResults:
return "TransferringArgsAndResults"
case .borrowingSwitch:
return "borrowing pattern matching"
}
}

Expand Down
23 changes: 22 additions & 1 deletion CodeGeneration/Sources/SyntaxSupport/KeywordSpec.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Comment on lines 21 to +22
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be nicer to make this an array of experimental features + update the doc comment to say that the keyword is enabled if any of the experimental features are enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this stuff is performance-sensitive so I wasn't sure about introducing a refcounted field into the structure just for a one-off case where we happen to need two features for the same keyword. I'm also hoping I can just take this out again once the .borrowingSwitch feature is enabled, which will hopefully occur in the nearish term.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just part of the code generation, so it’s not performance critical at all.


/// Indicates if the token kind is switched from being an identifier to a keyword in the lexer.
public let isLexerClassified: Bool
Expand Down Expand Up @@ -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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And then make the initializer take a variadic parameter: experimentalFeature: ExperimentalFeature…,

isLexerClassified: Bool = false
) {
self.name = name
self.experimentalFeature = experimentalFeature
self.experimentalFeature2 = experimentalFeature2
self.isLexerClassified = isLexerClassified
}
}
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
}

Expand Down Expand Up @@ -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
)
}
}
Expand Down
18 changes: 15 additions & 3 deletions Sources/SwiftParser/Expressions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
57 changes: 39 additions & 18 deletions Sources/SwiftParser/Patterns.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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
}
Expand Down Expand Up @@ -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(
Expand All @@ -228,7 +230,8 @@ extension Parser {
arena: self.arena
)
)
case nil:
case (.rhs(_), _)?,
nil:
break
}

Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
3 changes: 3 additions & 0 deletions Sources/SwiftParser/generated/ExperimentalFeatures.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
6 changes: 3 additions & 3 deletions Sources/SwiftParser/generated/Parser+TokenSpecSet.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading