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

makeUniqueName() produces tokens that cannot be used as closure arguments #2256

Open
grynspan opened this issue Oct 3, 2023 · 13 comments
Open
Labels
bug Something isn't working

Comments

@grynspan
Copy link
Contributor

grynspan commented Oct 3, 2023

Description

If I attempt to label closure arguments during macro expansion:

let arg0 = context.makeUniqueName("")
return """
{ \(arg0) in
  ...
}
"""

The compiler complains:

🛑 Inferred projection type 'Int' is not a property wrapper
🛑 Inferred projection type '() -> Int' is not a property wrapper

This means I can't reliably produce closure argument names, which is problematic if my macro expansion generates a closure and the macro is being used inside another closure:

@Test func foo() {
  let x = [2, 4, 6]
  x.forEach {
    #expect($0 % 2 == 0) // 🛑 Anonymous closure arguments cannot be used inside
                         // a closure that has explicit arguments; did you mean '$0'?
  }
}

Steps to Reproduce

See above.

@grynspan grynspan added the bug Something isn't working label Oct 3, 2023
@grynspan
Copy link
Contributor Author

grynspan commented Oct 3, 2023

See also swiftlang/swift#68475

@grynspan
Copy link
Contributor Author

grynspan commented Oct 3, 2023

I can repro with swift-DEVELOPMENT-SNAPSHOT-2023-10-02-a-osx, which I think means #68475 isn't a complete fix. My mistake, I was using a different toolchain.

@grynspan grynspan marked this as a duplicate of swiftlang/swift#68475 Oct 3, 2023
@grynspan grynspan closed this as completed Oct 3, 2023
@grynspan grynspan reopened this Oct 3, 2023
@grynspan
Copy link
Contributor Author

grynspan commented Oct 3, 2023

Having a day here… :) The issue with the spurious error is fixed, but I still can't use makeUniqueName() to generate my labels.

@ahoppen
Copy link
Member

ahoppen commented Oct 3, 2023

Tracked in Apple’s issue tracker as rdar://116402749

@ahoppen
Copy link
Member

ahoppen commented Oct 4, 2023

Reduced reproducer

public struct StringifyMacro: ExpressionMacro {
  public static func expansion(
    of node: some FreestandingMacroExpansionSyntax,
    in context: some MacroExpansionContext
  ) -> ExprSyntax {
    guard let argument = node.argumentList.first?.expression else {
      fatalError("compiler bug: the macro does not have any arguments")
    }
    let arg0 = context.makeUniqueName("")
    return """
      { \(arg0) in
        return \(argument)
      }(1)
      """
  }
}

Usage

_ = #stringify(a + b)

Expands to the following, which doesn’t compile

_ = { $s17mac_adsfjlkClient33_E5FD3D533106409A33F51D71F9BB5B8CLl9stringifyfMf_7__localfMu_ in
  return a + b
}(1)

Reproducing Package

@AppAppWorks
Copy link
Contributor

AppAppWorks commented Jul 15, 2024

@ahoppen It appears to me PluginMacroExpansionContext always receives from the host an expansionDiscriminator in the form of "$\(DemangledName)", that's why makeUniqueName always returns a "$"-prefixed string.

extension PluginMacroExpansionContext: MacroExpansionContext {
  /// Generate a unique name for use in the macro.
  public func makeUniqueName(_ providedName: String) -> TokenSyntax {
    // If provided with an empty name, substitute in something.
    let name = providedName.isEmpty ? "__local" : providedName

    // Grab a unique index value for this name.
    let uniqueIndex = uniqueNames[name, default: 0]
    uniqueNames[name] = uniqueIndex + 1

    // Start with the discriminator.
    var resultString = expansionDiscriminator

    // Mangle the name
    resultString += "\(name.count)\(name)"

    // Mangle the operator for unique macro names.
    resultString += "fMu"

    // Mangle the index.
    if uniqueIndex > 0 {
      resultString += "\(uniqueIndex - 1)"
    }
    resultString += "_"

    return TokenSyntax(.identifier(resultString), presence: .present)
  }

Can we just trim the leading "$" from expansionDiscriminator? In my understanding, $-prefixed identifiers are reserved for property wrapper projections, or should "$" be used for all compiler-synthesized identifiers?

@grynspan
Copy link
Contributor Author

The $ is added so that the generated name is guaranteed not to conflict with anything the developer might have written. In this context, the compiler's variable name validation rules may need to be relaxed to allow compiler-generated argument names.

@j-f1
Copy link
Contributor

j-f1 commented Jul 15, 2024

$-prefixed variable names are already valid here, as a way to construct a property wrapper-wrapped variable from its projected value. So somehow there’d need to be a way to distinguish those from the compiler-generated names. (And allow the compiler generated names to be used with the property wrapper syntax)

@AppAppWorks
Copy link
Contributor

the compiler's variable name validation rules may need to be relaxed to allow compiler-generated argument names.

@grynspan does it warrant a pitch?
If yes, I'd love to try on my first one!

@grynspan
Copy link
Contributor Author

As this is not my area, I wouldn't presume to answer that question. @DougGregor or @ahoppen probably know more than I do.

@AppAppWorks
Copy link
Contributor

AppAppWorks commented Jul 17, 2024

the compiler's variable name validation rules may need to be relaxed to allow compiler-generated argument names.

So somehow there’d need to be a way to distinguish those from the compiler-generated names. (And allow the compiler generated names to be used with the property wrapper syntax)

@ahoppen, drawing on the opinions of @grynspan and @j-f1, I think relaxing the use of compiler-generated $-prefixed identifiers in closures so that makeUniqueName can become useful may warrant an evolution pitch, do you think so?

@DougGregor
Copy link
Member

DougGregor commented Jul 18, 2024

I'd be fine with relaxing the use of compiler-generated $-prefixed identifiers in closures. A (short) evolution pitch would be reasonable to introduce this. It seems small enough that the Language Steering Group might be willing to call it a bug fix.

@AppAppWorks
Copy link
Contributor

Created [Pitch] Relax the use of compiler-generated $-prefixed identifiers in closures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants