Skip to content
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.

Sign up for GitHub

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

SwiftSyntax support for experimental @abi attribute #2882

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

SwiftSyntax support for experimental @abi attribute #2882

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
aa0b9a7
Conform NamedDeclSyntax to DeclSyntaxProtocol
beccadax Nov 5, 2024
df991ae
Introduce DeclGroupHeaderSyntax
beccadax Nov 5, 2024
2c57e11
[NFC] Refactor DeclarationKeyword
beccadax Nov 5, 2024
c335d65
Extract header nodes from DeclGroups
beccadax Oct 31, 2024
f77bb25
[NFC] Update uses of DeclGroups to use headers
beccadax Oct 31, 2024
cb0f4b9
Rework `HasTrailingMemberDeclBlock`
beccadax Nov 2, 2024
54ce286
Parse the `@abi` attribute in SwiftSyntax
beccadax Oct 31, 2024
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
30 changes: 30 additions & 0 deletions CodeGeneration/Sources/SyntaxSupport/AttributeNodes.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ public let ATTRIBUTE_NODES: [Node] = [
name: "documentationArguments",
kind: .node(kind: .documentationAttributeArgumentList)
),
Child(
name: "abiArguments",
kind: .node(kind: .abiAttributeArguments),
experimentalFeature: .abiAttribute
),
]),
documentation: """
The arguments of the attribute.
Expand Down Expand Up @@ -267,6 +272,31 @@ public let ATTRIBUTE_NODES: [Node] = [
]
),

Node(
Copy link
Member

Choose a reason for hiding this comment

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

Could you add an entry to the release notes for the new/updated nodes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At this point, the node is hidden behind an experimental feature. Should I still document it now?

Copy link
Member

Choose a reason for hiding this comment

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

I probably didn’t put the comment at the right place. I meant adding a release note entry for the nodes that change and that are not SPI, eg. the introduction of ClassDeclSyntax.header.

kind: .abiAttributeArguments,
base: .syntax,
experimentalFeature: .abiAttribute,
nameForDiagnostics: "ABI-providing declaration",
documentation: "The arguments of the '@abi' attribute",
children: [
Child(
name: "provider",
kind: ChildKind.nodeChoices(choices: [
Child(name: "associatedType", kind: .node(kind: .associatedTypeDecl)),
Child(name: "declGroup", kind: .node(kind: .declGroupHeader)),
Child(name: "deinitializer", kind: .node(kind: .deinitializerDecl)),
Child(name: "enumCase", kind: .node(kind: .enumCaseDecl)),
Child(name: "function", kind: .node(kind: .functionDecl)),
Child(name: "initializer", kind: .node(kind: .initializerDecl)),
Child(name: "subscript", kind: .node(kind: .subscriptDecl)),
Child(name: "typeAlias", kind: .node(kind: .typeAliasDecl)),
Child(name: "variable", kind: .node(kind: .variableDecl)),
Child(name: "unsupported", kind: .node(kind: .decl)),
Copy link
Member

Choose a reason for hiding this comment

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

Instead of having an unsupported case here, I think we should put the unsupported declaration into unexpected nodes. That way we can also generate a parser diagnostic for eg. @api(import Foo).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I do that, we probably still need a choice for .missingDecl, since we need to put something in the provider. Would that be preferred to unsupported?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, adding a choice for .missingDecl sounds like a good solution to me.

])
)
]
),

Node(
kind: .conventionAttributeArguments,
base: .syntax,
Expand Down
85 changes: 85 additions & 0 deletions CodeGeneration/Sources/SyntaxSupport/CommonNodes.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,13 +180,98 @@ public let COMMON_NODES: [Node] = [
parserFunction: "parseDeclaration"
),

Node(
kind: .declGroupHeader,
Copy link
Member

Choose a reason for hiding this comment

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

None of the other base nodes (DeclSyntax, ExprSyntax etc.) have any children and I think we will get crashes if any of the nodes whose base is .declGroupHeader are layout-incompatible with declGroupHeader. DeclGroupHeaderSyntax will assume that the first child in the node is attributes but that might not be the case for a node inheriting from .declGroupHeader.

That said, I would make the base of all header nodes .syntax and create a trait for the decl group headers instead.

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'm a little reluctant to do this because I get a fair bit of mileage out of having DeclGroupHeaderSyntax be a concrete type instead of an existential. For instance, both ABIAttributeSyntax.Provider and the modified version of HasTrailingMemberBlock use the concrete DeclGroupHeaderSyntax; the former could not easily use an existential because there's no way to constrain a child node to have a trait, and the latter because a concrete type is needed to provide a contextual type for the string literal.

(If it helps, the DeclGroupHeaderSyntax accessors don't directly access the children—they indirect through the existential so they use the leaf type's layout.)

If you can suggest another design that would still give me a concrete node which could contain any of the headers, I'd be open to that change.

Copy link
Member

Choose a reason for hiding this comment

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

Let me think about it some more. It might also become clearer to me once #2888 is merged and this PR is rebased so it doesn’t contain those changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @ahoppen , we now have the other PR merged. Do you have more specific suggestions in mind?

base: .syntax,
nameForDiagnostics: "declaration group header",
parserFunction: "parseDeclarationGroupHeader",
traits: [
"WithAttributes",
"WithModifiers",
],
children: [
Child(
name: "attributes",
kind: .collection(kind: .attributeList, collectionElementName: "Attribute", defaultsToEmpty: true),
nameForDiagnostics: "attributes"
),
Child(
name: "modifiers",
kind: .collection(kind: .declModifierList, collectionElementName: "Modifier", defaultsToEmpty: true),
nameForDiagnostics: "modifiers",
documentation: "Modifiers like `public` that are attached to the actor declaration."
),
Child(
name: "introducer",
kind: .token(choices: [
.keyword(.actor), .keyword(.class), .keyword(.enum), .keyword(.extension), .keyword(.protocol),
.keyword(.struct),
]),
documentation: "The token that introduces this declaration, eg. `class` for a class declaration."
),
Child(name: "inheritanceClause", kind: .node(kind: .inheritanceClause), isOptional: true),
Child(
name: "genericWhereClause",
kind: .node(kind: .genericWhereClause),
documentation:
"A `where` clause that places additional constraints on generic parameters like `where Element: Hashable`.",
isOptional: true
),
]
),

Node(
kind: .expr,
base: .syntax,
nameForDiagnostics: "expression",
parserFunction: "parseExpression"
),

Node(
kind: .missingDeclHeader,
base: .declGroupHeader,
nameForDiagnostics: "declaration group header",
documentation:
"In case the source code is missing a declaration group header, this node stands in place of the missing header.",
traits: [
"MissingNode",
"WithAttributes",
"WithModifiers",
],
children: [
Child(
name: "attributes",
kind: .collection(kind: .attributeList, collectionElementName: "Attribute", defaultsToEmpty: true),
documentation:
"If there were standalone attributes without a declaration to attach them to, the ``MissingDeclSyntax`` will contain these."
),
Child(
name: "modifiers",
kind: .collection(kind: .declModifierList, collectionElementName: "Modifier", defaultsToEmpty: true),
documentation:
"If there were standalone modifiers without a declaration to attach them to, the ``MissingDeclSyntax`` will contain these."
),
Child(
name: "placeholder",
kind: .token(choices: [.token(.identifier)], requiresLeadingSpace: false, requiresTrailingSpace: false),
documentation: """
A placeholder, i.e. `<#decl#>`, that can be inserted into the source code to represent the missing declaration.

This token should always have `presence = .missing`.
"""
),
Child(name: "inheritanceClause", kind: .node(kind: .inheritanceClause), isOptional: true),
Child(
name: "genericWhereClause",
kind: .node(kind: .genericWhereClause),
documentation:
"A `where` clause that places additional constraints on generic parameters like `where Element: Hashable`.",
isOptional: true
),

]
),

Node(
kind: .missingDecl,
base: .decl,
Expand Down
4 changes: 3 additions & 1 deletion CodeGeneration/Sources/SyntaxSupport/CompatibilityLayer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public struct CompatibilityLayer {
typeName: layoutNode.kind.rawValue,
initialChildren: layoutNode.children,
history: layoutNode.childHistory,
areRequirements: false
areRequirements: layoutNode.kind.isBase
)

deprecatedMembersByNode[node.syntaxNodeKind] = result
Expand Down Expand Up @@ -180,6 +180,8 @@ public struct CompatibilityLayer {
vars += children.filter { knownVars.insert($0).inserted }

// We don't create compatibility layers for protocol requirement inits.
// In theory, we *could* create compatibility layers for traits with `requiresInit: true`, but the only one we'd
// create so far is unnecessary and tricky to implement.
if !areRequirements {
initSignatures.append(InitSignature(children: children))
}
Expand Down
Loading