Parse _borrowing x contextually as a pattern binding when .borrowingSwitch feature is enabled.
#2487
Conversation
|
@swift-ci Please test |
jckarter
force-pushed
the
borrowing-switch-contextual-keyword
branch
from
32f793f to
1a7ff21
Compare
|
@swift-ci Please test |
And remove the `-disable-experimental-parser-round-trip` flag from borrowing switch tests now that the SwiftSyntax parser supports them with swiftlang/swift-syntax#2487.
jckarter
force-pushed
the
borrowing-switch-contextual-keyword
branch
from
1a7ff21 to
c0c9a19
Compare
|
@swift-ci Please test |
| public let experimentalFeature: ExperimentalFeature? | ||
| public let experimentalFeature2: ExperimentalFeature? |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
This is just part of the code generation, so it’s not performance critical at all.
| init( | ||
| _ name: String, | ||
| experimentalFeature: ExperimentalFeature, | ||
| or experimentalFeature2: ExperimentalFeature, |
There was a problem hiding this comment.
And then make the initializer take a variadic parameter: experimentalFeature: ExperimentalFeature…,
Sources/SwiftParser/Patterns.swift
Outdated
| var backtrack = lookahead() | ||
| if backtrack.shouldParsePatternBinding(introducer: introducer) { |
There was a problem hiding this comment.
We call it lookahead not backtrack in the new parser because the parser doesn’t actually backtrack. It clones itself on lookahead() and the clone looks ahead while the parser itself stays at the same position.
There was a problem hiding this comment.
I think all of these uses can be replaced by withLookahead, which I didn't know about. I used backtrack because it is used in several places to name a lookahead variable elsewhere in the codebase; are those other places needing updating?
% git grep 'backtrack ='
Sources/SwiftParser/Expressions.swift: var backtrack = self.lookahead()
Sources/SwiftParser/Expressions.swift: var backtrack = self.lookahead()
Sources/SwiftParser/Expressions.swift: let backtrack = self.lookahead()
Sources/SwiftParser/Expressions.swift: var backtrack = lookahead()
Sources/SwiftParser/Expressions.swift: var backtrack = self.lookahead()
Sources/SwiftParser/Expressions.swift: var backtrack = self.lookahead()
Sources/SwiftParser/Expressions.swift: var backtrack = self.lookahead()
Sources/SwiftParser/Lookahead.swift: var backtrack = self.lookahead()
Sources/SwiftParser/Names.swift: var backtrack = self.lookahead()
Sources/SwiftParser/Names.swift: var backtrack = self.lookahead()
Sources/SwiftParser/Patterns.swift: var backtrack = self.lookahead()
Sources/SwiftParser/Patterns.swift: var backtrack = lookahead()
Sources/SwiftParser/Types.swift: var backtrack = self.lookahead()
Sources/SwiftParser/Types.swift: var backtrack = self.lookahead()
There was a problem hiding this comment.
Huh, yeah, they should probably all be updated. Probably still a leftover from the migration from C++. Filed #2494
jckarter
force-pushed
the
borrowing-switch-contextual-keyword
branch
from
c0c9a19 to
007c84e
Compare
|
@swift-ci Please test |
|
@ahoppen Thanks for the tip about |
jckarter
force-pushed
the
borrowing-switch-contextual-keyword
branch
from
007c84e to
6377fc9
Compare
|
@swift-ci Please test |
1 similar comment
|
@swift-ci Please test |
…ngSwitch` 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.
jckarter
force-pushed
the
borrowing-switch-contextual-keyword
branch
from
6377fc9 to
43c13ca
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
The existing parsing under
.referenceBindingsparses this (along with_mutatingand_consuming) as unconditional keywords, which would be a source break we can't ship. Narrow the behavior so that_borrowingis only parsed immediately before an identifier in a pattern.