Add support for lifetime type specifiers #2433
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ahoppen
force-pushed
the
ahoppen/allow-multiple-type-specifiers
branch
3 times, most recently
from
4dd7bc7 to
e5c0834
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
bnbarham
reviewed
Jan 22, 2024
Sources/SwiftSyntax/generated/syntaxNodes/SyntaxNodesTUVWXYZ.swift
Outdated
Show resolved
Hide resolved
ahoppen
force-pushed
the
ahoppen/allow-multiple-type-specifiers
branch
from
e5c0834 to
366fa95
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
ahoppen
force-pushed
the
ahoppen/allow-multiple-type-specifiers
branch
from
366fa95 to
b627323
Compare
ahoppen
added a commit
to ahoppen/swift
that referenced
this pull request
Feb 6, 2024
ahoppen
added a commit
to ahoppen/swift
that referenced
this pull request
Feb 6, 2024
|
@swift-ci Please test |
ahoppen
force-pushed
the
ahoppen/allow-multiple-type-specifiers
branch
from
b627323 to
685768d
Compare
|
@swift-ci Please test macOS |
ahoppen
force-pushed
the
ahoppen/allow-multiple-type-specifiers
branch
3 times, most recently
from
5a0ecc9 to
4a1ba3b
Compare
ahoppen
changed the title
|
@swift-ci Please test macOS |
ahoppen
force-pushed
the
ahoppen/allow-multiple-type-specifiers
branch
from
4a1ba3b to
81bf075
Compare
|
@swift-ci Please test |
meg-gupta
reviewed
Mar 1, 2024
There was a problem hiding this comment.
Lifetime dependence changes lgtm. I added some minor comments about tests. That + changes for initializers can be done as a follow up.
Thanks for this PR! With this we are almost close to get rid of -disable-experimental-parser-round-trip in our lifetime dependence tests!
| experimentalFeatures: [.nonescapableTypes] | ||
| ) | ||
|
|
||
| assertParse("func foo() -> _borrow(0) X", diagnostics: [], experimentalFeatures: [.nonescapableTypes]) |
There was a problem hiding this comment.
can you add a failure test with a -ve integer ?
There was a problem hiding this comment.
Added a test case for _borrow(-1)
| @@ -325,4 +325,59 @@ final class TypeTests: ParserTestCase { | |||
| assertParse("func foo1(_ a: _const borrowing String) {}") | |||
| assertParse("func foo2(_ a: borrowing _const String) {}") | |||
| } | |||
|
|
|||
| func testLifetimeSpecifier() { | |||
| assertParse("func foo() -> _borrow(x) X", experimentalFeatures: [.nonescapableTypes]) | |||
There was a problem hiding this comment.
can you add a failure test without the experimental feature?
ahoppen
force-pushed
the
ahoppen/allow-multiple-type-specifiers
branch
from
81bf075 to
6c36cec
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
|
Here’s an abbreviated summary of the new nodes. public struct AttributedTypeSyntax {
/// A list of specifiers that can be attached to the type, such as `inout`, `isolated`, or `consuming`.
public var specifiers: TypeSpecifierListSyntax
/// A list of attributes that can be attached to the type, such as `@escaping`.
public var attributes: AttributeListSyntax
/// The type to with the specifiers and attributes are applied.
public var baseType: TypeSyntax
}
/// ### Children
///
/// (``SimpleTypeSpecifierSyntax`` | ``LifetimeTypeSpecifierSyntax``) `*`
///
/// ### Contained in
///
/// - ``AttributedTypeSyntax``.``AttributedTypeSyntax/specifiers``
public struct TypeSpecifierListSyntax {
public enum Element: SyntaxChildChoices {
case `simpleTypeSpecifier`(SimpleTypeSpecifierSyntax)
case `lifetimeTypeSpecifier`(LifetimeTypeSpecifierSyntax)
}
}
/// A specifier that can be attached to a type to eg. mark a parameter as `inout` or `consuming`
///
/// ### Children
///
/// - `specifier`: (`inout` | `__shared` | `__owned` | `isolated` | `_const` | `borrowing` | `consuming` | `transferring` | `_resultDependsOn`)
///
/// ### Contained in
///
/// - ``TypeSpecifierListSyntax``
public struct SimpleTypeSpecifierSyntax {
/// The specifier token that's attached to the type.
///
/// ### Tokens
///
/// For syntax trees generated by the parser, this is guaranteed to be one of the following kinds:
/// - `inout`
/// - `__shared`
/// - `__owned`
/// - `isolated`
/// - `_const`
/// - `borrowing`
/// - `consuming`
/// - `transferring`
/// - `_resultDependsOn`
public var specifier: TokenSyntax
}
/// A specifier that specifies function parameter on whose lifetime a type depends
///
/// ### Children
///
/// - `specifier`: (`_copy` | `_consume` | `_borrow` | `_mutate`)
/// - `arguments`: ``LifetimeSpecifierArgumentsSyntax``
///
/// ### Contained in
///
/// - ``TypeSpecifierListSyntax``
public struct LifetimeTypeSpecifierSyntax {
/// The specifier token that's attached to the type.
///
/// ### Tokens
///
/// For syntax trees generated by the parser, this is guaranteed to be one of the following kinds:
/// - `_copy`
/// - `_consume`
/// - `_borrow`
/// - `_mutate`
public var specifier: TokenSyntax
public var arguments: LifetimeSpecifierArgumentsSyntax
}
/// ### Children
///
/// ``LifetimeSpecifierArgumentSyntax`` `*`
///
/// ### Contained in
///
/// - ``LifetimeSpecifierArgumentsSyntax``.``LifetimeSpecifierArgumentsSyntax/arguments``
public struct LifetimeSpecifierArgumentListSyntax {
public typealias Element = LifetimeSpecifierArgumentSyntax
}
/// An optional argument passed to a type parameter.
///
/// ### Example
/// `borrow(data)` in `func foo(data: Array<Item>) -> borrow(data) ComplexReferenceType`
///
/// ### Children
///
/// - `leftParen`: `(`
/// - `arguments`: ``LifetimeSpecifierArgumentListSyntax``
/// - `rightParen`: `)`
///
/// ### Contained in
///
/// - ``LifetimeTypeSpecifierSyntax``.``LifetimeTypeSpecifierSyntax/arguments``
public struct LifetimeSpecifierArgumentsSyntax {
/// ### Tokens
///
/// For syntax trees generated by the parser, this is guaranteed to be `(`.
public var leftParen: TokenSyntax
/// The function parameters that the lifetime of the annotated type depends on.
public var arguments: LifetimeSpecifierArgumentListSyntax
/// ### Tokens
///
/// For syntax trees generated by the parser, this is guaranteed to be `)`.
public var rightParen: TokenSyntax
}
/// A single argument that can be added to a lifetime specifier like `borrow`, `mutate`, `consume` or `copy`.
///
/// ### Example
/// `data` in `func foo(data: Array<Item>) -> borrow(data) ComplexReferenceType`
///
/// ### Children
///
/// - `parameter`: (`<identifier>` | `self` | `<integerLiteral>`)
/// - `trailingComma`: `,`?
///
/// ### Contained in
///
/// - ``LifetimeSpecifierArgumentListSyntax``
public struct LifetimeSpecifierArgumentSyntax {
/// The parameter on which the lifetime of this type depends.
///
/// This can be an identifier referring to an external parameter name, an integer literal to refer to an unnamed
/// parameter or `self` if the type's lifetime depends on the object the method is called on.
///
/// ### Tokens
///
/// For syntax trees generated by the parser, this is guaranteed to be one of the following kinds:
/// - `<identifier>`
/// - `self`
/// - `<integerLiteral>`
public var parameter: TokenSyntax
/// ### Tokens
///
/// For syntax trees generated by the parser, this is guaranteed to be `,`.
public var trailingComma: TokenSyntax?
} |
hamishknight
approved these changes
Mar 8, 2024
Comment on lines
498
to
502
| Node( | ||
| kind: .lifetimeSpecifierArgument, | ||
| base: .syntax, | ||
| nameForDiagnostics: nil, |
There was a problem hiding this comment.
Should the lifetime specifier nodes be marked experimental and therefore SPI?
|
Yeah, that representation looks fine to me. |
ahoppen
force-pushed
the
ahoppen/allow-multiple-type-specifiers
branch
2 times, most recently
from
9c33644 to
8da6dba
Compare
|
@swift-ci Please test |
Having multiple type specifiers on a type is valid in the C++ parser but not in the Swift parser. rdar://118125715
… generated `TypeSpecifierSyntax.SpecifierOptions` The two `TokenSpecSet`s had already diverged. Instead of manually maintaining the `TypeSpecifier` spec set, we should allow all keywors specified in the syntax tree definition.
…f as docc links Docc doesn’t like references to SPI types because they are not part of the public API.
ahoppen
force-pushed
the
ahoppen/allow-multiple-type-specifiers
branch
from
8da6dba to
3df63e8
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Having multiple type specifiers on a type is valid in the C++ parser but not in the Swift parser. Also add support for lifetime specifiers that can have arguments, like
borrow(data).rdar://118125715
While at it, replace the manually maintained
TokenSpecSetTypeSpecifierby the generatedTypeSpecifierSyntax.SpecifierOptionsThe two
TokenSpecSets had already diverged. Instead of manually maintaining theTypeSpecifierspec set, we should allow all keywors specified in the syntax tree definition.