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.

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

Merge macro examples from ExamplePlugin target into MacroExamples #2222

Merged

Conversation

Matejkob
Copy link
Contributor

@Matejkob Matejkob commented Sep 20, 2023

Resolves #2165

Copy link
Contributor Author

@Matejkob Matejkob left a comment

Choose a reason for hiding this comment

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

I've merged almost every macro example. I have some doubts about the quality of certain merged macros and whether they serve as good examples, but I'll leave that to your assessment.

Comment on lines -117 to -132
/// Add 'didSet' printing the new value.
struct DidSetPrintMacro: AccessorMacro {
static func expansion(
of node: AttributeSyntax,
providingAccessorsOf declaration: some DeclSyntaxProtocol,
in context: some MacroExpansionContext
) throws -> [AccessorDeclSyntax] {
guard
let identifier = declaration.as(VariableDeclSyntax.self)?.bindings.first?.pattern.as(IdentifierPatternSyntax.self)?.identifier
else {
return []
}

return ["didSet { print(\(identifier)) }"]
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Accessor macro cannot add an observable accessor so this example doesn't work so I removed it.

Comment on lines -134 to -145
/// 'print(<arg>)'.
struct PrintAnyMacro: CodeItemMacro {
static func expansion(
of node: some FreestandingMacroExpansionSyntax,
in context: some MacroExpansionContext
) throws -> [CodeBlockItemSyntax] {
guard let expr = node.arguments.first?.expression else {
return []
}
return ["print(\(expr))"]
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code item macro is still behind a feature flag, so I've decided not to merge it into the official macro examples yet to avoid confusing users.


let counter = Counter()

// print("Peer value with suffix name for \(Counter.self): \(String(describing: Counter_peer))")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have any idea why Counter_peer con not be accessed here?

Copy link
Member

Choose a reason for hiding this comment

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

That appears to be a bug. @DougGregor is this a known issue that name lookup doesn’t work for peers of local types?

If I move the following to top-level it does work.

@PeerValueWithSuffixName
actor Counter {
  var value = 0
}

Copy link
Member

Choose a reason for hiding this comment

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

OK, looks like it might be intentional that peer macro can’t introduce declarations on a local scope. If so, the error message should be improved. Filed swiftlang/swift#68661

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Alex

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

I’m not sure if all of these examples are all that valuable. I think the ones that do provide additional value are:

  • FuncUniqueMacro since we don’t have any declaration macro examples right now
  • SendableExtensionMacro since we don’t have any extension macro examples right now
  • MemberDeprecatedMacro seems like a nice example and is a lot simpler than our existing MemberAttribute macro
  • PeerValueWithSuffixNameMacro also seems useful since it’s a very short example of a peer macro and all our current examples are a lot more complicated

I think the following aren’t really useful and I would just delete them:

  • EchoExpressionMacro since it doesn’t do anything and we have plenty expression macro examples already
  • EquatableConformanceMacro since conformance macros aren’t really a thing anymore
  • MetadataMacro: I think CaseDetectionMacro shows similar ideas and is a more realistic example

@rintaro @bnbarham Do you have opinions here as well?

Examples/Sources/MacroExamples/Implementation/Plugin.swift Outdated Show resolved Hide resolved

let counter = Counter()

// print("Peer value with suffix name for \(Counter.self): \(String(describing: Counter_peer))")
Copy link
Member

Choose a reason for hiding this comment

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

That appears to be a bug. @DougGregor is this a known issue that name lookup doesn’t work for peers of local types?

If I move the following to top-level it does work.

@PeerValueWithSuffixName
actor Counter {
  var value = 0
}

@bnbarham
Copy link
Contributor

I’m not sure if all of these examples are all that valuable. I think the ones that do provide additional value are:

  • FuncUniqueMacro since we don’t have any declaration macro examples right now
  • SendableExtensionMacro since we don’t have any extension macro examples right now
  • MemberDeprecatedMacro seems like a nice example and is a lot simpler than our existing MemberAttribute macro
  • PeerValueWithSuffixNameMacro also seems useful since it’s a very short example of a peer macro and all our current examples are a lot more complicated

I think the following aren’t really useful and I would just delete them:

  • EchoExpressionMacro since it doesn’t do anything and we have plenty expression macro examples already
  • EquatableConformanceMacro since conformance macros aren’t really a thing anymore
  • MetadataMacro: I think CaseDetectionMacro shows similar ideas and is a more realistic example

@rintaro @bnbarham Do you have opinions here as well?

That list sounds mostly reasonable to me, though the reasoning for not including EquatableConformanceMacro is more because SendableExtensionMacro is basically the exact same but with a better implementation.

@Matejkob
Copy link
Contributor Author

Speak of SendableExtensionMacro:
image
Do you know what is goin' on?

@Matejkob Matejkob force-pushed the merge-example-plung-target-into-macro-examples branch from 80dddfa to 067236d Compare September 21, 2023 20:47
@ahoppen
Copy link
Member

ahoppen commented Sep 21, 2023

Do you know what is goin' on?

Oh, the macro expansion is considered a different source file in the compiler. Could you file a bug at https://github.com/apple/swift/issues?

I would suggest we just remove that example as well. We should add an example for an extension macro but I we could do that in a follow-up PR.

@bnbarham
Copy link
Contributor

I would suggest we just remove that example as well. We should add an example for an extension macro but I we could do that in a follow-up PR.

Could just rename to to Equatable 😅

@Matejkob
Copy link
Contributor Author

I would suggest we just remove that example as well. We should add an example for an extension macro but I we could do that in a follow-up PR.

Could just rename to to Equatable 😅

That could work! 😅

@Matejkob Matejkob force-pushed the merge-example-plung-target-into-macro-examples branch from 067236d to 0c044f7 Compare September 22, 2023 17:58
@bnbarham
Copy link
Contributor

@swift-ci please test

@ahoppen
Copy link
Member

ahoppen commented Sep 23, 2023

@swift-ci Please test Windows

@ahoppen ahoppen merged commit fee7838 into swiftlang:main Sep 23, 2023
3 checks passed
ahoppen added a commit to ahoppen/swift-syntax that referenced this pull request Sep 25, 2023
plemarquand added a commit to plemarquand/swift-syntax that referenced this pull request Oct 7, 2024
Fixes up several broken links in the documentation:
- ExamplePlugin was rolled in to the MacroExamples in swiftlang#2222 but the
  link was not updated.
- A link the changelog pointed to sourcekit-lsp when it should have been
  swift-syntax.
- The Parsing Basics document has moved.
plemarquand added a commit to plemarquand/swift-syntax that referenced this pull request Oct 7, 2024
Fixes up several broken links in the documentation:
- ExamplePlugin was rolled in to the MacroExamples in swiftlang#2222 but the
  link was not updated.
- A link the changelog pointed to sourcekit-lsp when it should have been
  swift-syntax.
- The Parsing Basics document has moved.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorporate the ExamplePlugin target into the MacroExamples target
3 participants