`assertMacroExpansion` should emit an error if member macro is applied to declaration that can’t have members

[ahoppen]

When applying an attached member macro to e.g. a variable, assertMacroExpansion will currently happily swallow the attribute. It should, however, emit an error that member macros can’t be applied to variables (specifically, if the declaration is not a DeclGroupSyntax). Ie. the following test case should emit an error.

func testMemberMacroOnVar() {
  struct TestMacro: MemberMacro {
    static func expansion(
      of node: AttributeSyntax,
      providingMembersOf declaration: some DeclGroupSyntax,
      conformingTo protocols: [TypeSyntax],
      in context: some MacroExpansionContext
    ) throws -> [DeclSyntax] {
      return []
    }
  }

  assertMacroExpansion(
    """
    @Test var x: Int
    """,
    expandedSource: """
      var x: Int
      """,
    macros: [
      "Test": TestMacro.self
    ]
  )
}

rdar://115562663

[Rajveer100]

@ahoppen It would be great if you could further describe a little about swift-syntax and this issue as well?

If I remember correctly, macros were introduced this year (WWDC 2023) from Swift 5.9 and earlier it was always hidden behind the implementation of many Swift features?

I am digging down to learn more about it, let me know if there any specific resources w.r.t this issue.

[Rajveer100]

@ahoppen I am facing an issue with the following command:

> swift run --package-path CodeGeneration

Error:

Fetching https://github.com/apple/swift-argument-parser.git
Fetched https://github.com/apple/swift-argument-parser.git (2.24s)
Computing version for https://github.com/apple/swift-argument-parser.git
Computed https://github.com/apple/swift-argument-parser.git at 1.2.3 (0.61s)
Creating working copy for https://github.com/apple/swift-argument-parser.git
Working copy of https://github.com/apple/swift-argument-parser.git resolved at 1.2.3
Building for debugging...
[272/272] Linking generate-swift-syntax
Build complete! (17.20s)
dyld[21618]: Library not loaded: @rpath/libc++.1.dylib <----------
  Referenced from: <F5104EA3-23D6-34EC-B647-516182F5AFE8> ../swift-project/swift-syntax/CodeGeneration/.build/arm64-apple-macosx/debug/generate-swift-syntax
  Reason: tried: '/usr/lib/swift/libc++.1.dylib' (no such file, not in dyld cache), '/System/Volumes/Preboot/Cryptexes/OS/usr/lib/swift/libc++.1.dylib' (no such file), '/Users/rajveersingh/GitHub-OpenSource/swift-project/swift-syntax/CodeGeneration/.build/arm64-apple-macosx/debug/libc++.1.dylib' (no such file), '/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift-5.5/macosx/libc++.1.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift-5.5/macosx/libc++.1.dylib' (no such file), '/usr/lib/swift/libc++.1.dylib' (no such file, not in dyld cache), '/System/Volumes/Preboot/Cryptexes/OS/usr/lib/swift/libc++.1.dylib' (no such file), '/Users/rajveersingh/GitHub-OpenSource/swift-project/swift-syntax/CodeGeneration/.build/arm64-apple-macosx/debug/libc++.1.dylib' (no such file), '/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift-5.5/macosx/libc++.1.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift-5.5/macosx/libc++.1.dylib' (no such file)
zsh: abort      swift run --package-path CodeGeneration

This issue regarding libc++ and dyld has been there for a while, not sure what caused it.

[It's also occurring in JetBrains IDEs in Debug Mode (normal mode works), but this was fixed by adding an ENVIRONMENT VARIABLE for DYLD_LIBRARY_PATH.] (Not related to this issue, but it's exactly the same)

I have also tried adding a global env to zsh and bash but doesn't resolve.

PS: I have added a Swift Forums post as well.

[ahoppen]

Looks like you figured out the libc++ issue with @hamishknight on the forum thread 👍🏽

If you have any concrete questions regarding this issue, I’m happy to answer them, otherwise I think the issue description describes the issue fairly well. We have some general documentation of how swift-syntax works hosted on https://swiftpackageindex.com/apple/swift-syntax/main/documentation/swiftsyntax

[Rajveer100]

If I understand correctly, we need to add a check inside the MemberMacro expansion to emit a diagnostic whenever we attach a member macro to a variable.

As an example if I created my custom macro that's only constrained for struct, enum, etc, we should emit an error for other cases appropriately to avoid unexpected behaviour. So we potentially add a failing test case first and then make the changes and check again for it to pass.

If my above intuition is right, could you highlight the appropriate file directories we are targeting currently? Regarding tests I suppose they all go in Tests?

[ahoppen]

If my above intuition is right, could you highlight the appropriate file directories we are targeting currently? Regarding tests I suppose they all go in Tests?

The check should be in MacroSystem.

I would suggest that you add the above test case to your swift-syntax checkout. If you run it, it should fail. And then you can debug how to fix it.

[Rajveer100]

In MacroExpansion, I see the case for MemberMacro in expandAttachedMacroWithoutCollapsing which is called via MacroSystem during expansion.

So to check for declaration, do we guard declarationNode and throw MacroExpansionError in case it's a VariableDeclSyntax?

Also, I have added the snippet test as you described under MemberMacroTests where I see the list of various functions and before making any change, no errors are thrown when running tests which I presume is the intended issue as this is the case that is not being caught, right?

[Rajveer100]

Is the following snippet the right way to do the check, as at the moment many other tests also fail due to this addition:

guard declGroup.syntaxNodeType == VariableDeclSyntax.self else {
  throw MacroExpansionError.declarationNotIdentified
}

DeclGroupSyntax check is already present before this.

[Rajveer100]

@ahoppen Just wanted to know if I am on the right track based on the above context.

[ahoppen]

From your description, that seems like the right direction but it’s usually easier to tell if you open a PR and there’s code to look at.