From 61c9d202782b7f76e35f31cd338f600183fcd154 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 9 Jun 2023 10:39:10 -0700 Subject: [PATCH 1/6] [Macros] Only freestanding expression macros can have a non-Void result type Fixes rdar://108871352. (cherry picked from commit 759f52066e9c703cc8ce3abc25dd5026ded00a99) --- include/swift/AST/DiagnosticsSema.def | 8 ++++++++ lib/AST/Decl.cpp | 6 ++++++ lib/Sema/TypeCheckDeclPrimary.cpp | 17 +++++++++++++++++ test/Macros/macros_diagnostics.swift | 13 ++++++++++++- 4 files changed, 43 insertions(+), 1 deletion(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 81a8f58676872..35e16935d0f0f 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -7052,6 +7052,14 @@ ERROR(macro_in_nested,none, ERROR(macro_without_role,none, "macro %0 must declare its applicable roles via '@freestanding' or @attached'", (DeclName)) +ERROR(macro_result_type_cannot_be_used,none, + "only a freestanding expression macro can produce a result of type %0", + (Type)) +NOTE(macro_remove_result_type,none, + "remove the result type if the macro does not produce a value", + ()) +NOTE(macro_make_freestanding_expression,none, + "make this macro a freestanding expression macro", ()) ERROR(macro_expansion_missing_pound,none, "expansion of macro %0 requires leading '#'", (DeclName)) ERROR(macro_expansion_missing_arguments,none, diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index f40ad741ed6b7..2a37eb3e7a8b8 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -3357,12 +3357,18 @@ TypeRepr *ValueDecl::getResultTypeRepr() const { returnRepr = FD->getResultTypeRepr(); } else if (auto *SD = dyn_cast(this)) { returnRepr = SD->getElementTypeRepr(); + } else if (auto *MD = dyn_cast(this)) { + returnRepr = MD->resultType.getTypeRepr(); } return returnRepr; } TypeRepr *ValueDecl::getOpaqueResultTypeRepr() const { + // FIXME: Macros don't allow opaque result types yet. + if (isa(this)) + return nullptr; + auto *returnRepr = this->getResultTypeRepr(); auto *dc = getDeclContext(); diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp index febd446ceed92..36f055daad036 100644 --- a/lib/Sema/TypeCheckDeclPrimary.cpp +++ b/lib/Sema/TypeCheckDeclPrimary.cpp @@ -2061,6 +2061,23 @@ class DeclChecker : public DeclVisitor { break; } } + + // If the macro has a (non-Void) result type, it must have the freestanding + // expression role. Other roles cannot have result types. + if (auto resultTypeRepr = MD->getResultTypeRepr()) { + if (!MD->getMacroRoles().contains(MacroRole::Expression) && + !MD->getResultInterfaceType()->isEqual(Ctx.getVoidType())) { + auto resultType = MD->getResultInterfaceType(); + Ctx.Diags.diagnose( + MD->arrowLoc, diag::macro_result_type_cannot_be_used, resultType) + .highlight(resultTypeRepr->getSourceRange()); + Ctx.Diags.diagnose(MD->arrowLoc, diag::macro_make_freestanding_expression) + .fixItInsert(MD->getAttributeInsertionLoc(false), + "@freestanding(expression)\n"); + Ctx.Diags.diagnose(MD->arrowLoc, diag::macro_remove_result_type) + .fixItRemove(SourceRange(MD->arrowLoc, resultTypeRepr->getEndLoc())); + } + } } void visitMacroExpansionDecl(MacroExpansionDecl *MED) { diff --git a/test/Macros/macros_diagnostics.swift b/test/Macros/macros_diagnostics.swift index 429ac87f77fc5..1955d9f6a76ec 100644 --- a/test/Macros/macros_diagnostics.swift +++ b/test/Macros/macros_diagnostics.swift @@ -46,7 +46,7 @@ internal struct X { } // expected-note{{type declared here}} // expected-warning@-1{{external macro implementation type}} struct ZZZ { - macro m5() -> Int = #externalMacro(module: "BuiltinMacros", type: "Blah") + macro m5() = #externalMacro(module: "BuiltinMacros", type: "Blah") // expected-error@-1{{macro 'm5()' can only be declared at file scope}} // expected-error@-2{{macro 'm5()' must declare its applicable roles}} // expected-warning@-3{{external macro implementation type}} @@ -200,3 +200,14 @@ struct SomeType { // expected-error@-2{{use of protocol 'Hashable' as a type must be written 'any Hashable'}} // expected-error@-3{{external macro implementation type}} } + + + +@freestanding(declaration) macro nonExpressionReturnsInt(_: T) -> Int = #externalMacro(module: "A", type: "B") +// expected-warning@-1{{external macro implementation type}} +// expected-error@-2{{only a freestanding expression macro can produce a result of type 'Int'}} +// expected-note@-3{{make this macro a freestanding expression macro}}{{1-1=@freestanding(expression)\n}} +// expected-note@-4{{remove the result type if the macro does not produce a value}}{{67-74=}} + +@freestanding(declaration) macro nonExpressionReturnsVoid(_: T) -> Void = #externalMacro(module: "A", type: "B") +// expected-warning@-1{{external macro implementation type}} From 9fd3fc623c6b2c2037dbdbfc013b0364d7e04257 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 9 Jun 2023 12:59:56 -0700 Subject: [PATCH 2/6] [Macros] Provide the freestanding macro role for expansion operations. The compiler knows (from a macro declaration) what freestanding macro role a macro implementation is expected to implement. Pass that through to the macro expansion code itself, rather than guessing based on the protocol conformances of the implementation type. We already use this approach with attached macros, so this is more of the same. Eliminates a crash and improves diagnostics when the freestanding macro role and its implementation are out of sync, fixing rdar://110418969. (cherry picked from commit 3c04cff8dd739fe3728808a092542020504065c8) --- lib/ASTGen/Sources/ASTGen/Macros.swift | 21 ++++++++++++++++ .../Sources/ASTGen/PluginMessages.swift | 2 ++ lib/IDETool/SyntacticMacroExpansion.cpp | 2 +- lib/Sema/TypeCheckMacros.cpp | 13 ++++++++-- .../Inputs/syntax_macro_definitions.swift | 20 ++++++++++++++++ test/Macros/macro_expand.swift | 24 +++++++++++++++++++ test/Macros/macro_plugin_basic.swift | 4 ++-- test/Macros/macro_plugin_error.swift | 6 ++--- test/Macros/macro_plugin_server.swift | 6 ++--- 9 files changed, 87 insertions(+), 11 deletions(-) diff --git a/lib/ASTGen/Sources/ASTGen/Macros.swift b/lib/ASTGen/Sources/ASTGen/Macros.swift index 12fbaaf20c420..1055dac8b8875 100644 --- a/lib/ASTGen/Sources/ASTGen/Macros.swift +++ b/lib/ASTGen/Sources/ASTGen/Macros.swift @@ -67,6 +67,8 @@ extension MacroRole { case 0x10: self = .member case 0x20: self = .peer case 0x40: self = .conformance + case 0x80: self = .codeItem + default: fatalError("unknown macro role") } } @@ -414,6 +416,7 @@ func expandFreestandingMacro( macroKind: UInt8, discriminatorText: UnsafePointer, discriminatorTextLength: Int, + rawMacroRole: UInt8, sourceFilePtr: UnsafeRawPointer, sourceLocationPtr: UnsafePointer?, expandedSourcePointer: UnsafeMutablePointer?>, @@ -446,11 +449,13 @@ func expandFreestandingMacro( ) let discriminator = String(decoding: discriminatorBuffer, as: UTF8.self) + let macroRole = MacroRole(rawMacroRole: rawMacroRole) let expandedSource: String? switch MacroPluginKind(rawValue: macroKind)! { case .InProcess: expandedSource = expandFreestandingMacroInProcess( macroPtr: macroPtr, + macroRole: macroRole, diagEnginePtr: diagEnginePtr, expansionSyntax: expansion, sourceFilePtr: sourceFilePtr, @@ -458,6 +463,7 @@ func expandFreestandingMacro( case .Executable: expandedSource = expandFreestandingMacroIPC( macroPtr: macroPtr, + macroRole: macroRole, diagEnginePtr: diagEnginePtr, expansionSyntax: expansion, sourceFilePtr: sourceFilePtr, @@ -485,6 +491,7 @@ func expandFreestandingMacro( func expandFreestandingMacroIPC( macroPtr: UnsafeRawPointer, + macroRole: MacroRole, diagEnginePtr: UnsafeMutablePointer, expansionSyntax: FreestandingMacroExpansionSyntax, sourceFilePtr: UnsafePointer, @@ -502,9 +509,21 @@ func expandFreestandingMacroIPC( let macro = macroPtr.assumingMemoryBound(to: ExportedExecutableMacro.self).pointee + // Map the macro role. + let pluginMacroRole: PluginMessage.MacroRole + switch macroRole { + case .accessor, .member, .memberAttribute, .peer, .conformance: + preconditionFailure("unhandled macro role for freestanding macro") + + case .expression: pluginMacroRole = .expression + case .declaration: pluginMacroRole = .freeStandingDeclaration + case .codeItem: pluginMacroRole = .codeItem + } + // Send the message. let message = HostToPluginMessage.expandFreestandingMacro( macro: .init(moduleName: macro.moduleName, typeName: macro.typeName, name: macroName), + macroRole: pluginMacroRole, discriminator: discriminator, syntax: PluginMessage.Syntax(syntax: Syntax(expansionSyntax), in: sourceFilePtr)!) do { @@ -541,6 +560,7 @@ func expandFreestandingMacroIPC( func expandFreestandingMacroInProcess( macroPtr: UnsafeRawPointer, + macroRole: MacroRole, diagEnginePtr: UnsafeMutablePointer, expansionSyntax: FreestandingMacroExpansionSyntax, sourceFilePtr: UnsafePointer, @@ -580,6 +600,7 @@ func expandFreestandingMacroInProcess( return SwiftSyntaxMacroExpansion.expandFreestandingMacro( definition: macro, + macroRole: macroRole, node: node, in: context ) diff --git a/lib/ASTGen/Sources/ASTGen/PluginMessages.swift b/lib/ASTGen/Sources/ASTGen/PluginMessages.swift index df67a178f43c3..cd058797f90f0 100644 --- a/lib/ASTGen/Sources/ASTGen/PluginMessages.swift +++ b/lib/ASTGen/Sources/ASTGen/PluginMessages.swift @@ -20,6 +20,7 @@ internal enum HostToPluginMessage: Codable { /// Expand a '@freestanding' macro. case expandFreestandingMacro( macro: PluginMessage.MacroReference, + macroRole: PluginMessage.MacroRole? = nil, discriminator: String, syntax: PluginMessage.Syntax ) @@ -91,6 +92,7 @@ internal enum PluginToHostMessage: Codable { case member case peer case conformance + case codeItem } struct SourceLocation: Codable { diff --git a/lib/IDETool/SyntacticMacroExpansion.cpp b/lib/IDETool/SyntacticMacroExpansion.cpp index afcffa672721b..e7a16e7bb62db 100644 --- a/lib/IDETool/SyntacticMacroExpansion.cpp +++ b/lib/IDETool/SyntacticMacroExpansion.cpp @@ -412,7 +412,7 @@ void SyntacticMacroExpansionInstance::expand( SourceFile *SF, const MacroExpansionSpecifier &expansion, SourceEditConsumer &consumer) { - // Find the expansion at 'expantion.offset'. + // Find the expansion at 'expansion.offset'. MacroExpansionFinder expansionFinder( SourceMgr, SourceMgr.getLocForOffset(*SF->getBufferID(), expansion.offset)); diff --git a/lib/Sema/TypeCheckMacros.cpp b/lib/Sema/TypeCheckMacros.cpp index b607f918842c4..4e9343e359f78 100644 --- a/lib/Sema/TypeCheckMacros.cpp +++ b/lib/Sema/TypeCheckMacros.cpp @@ -66,7 +66,8 @@ extern "C" ptrdiff_t swift_ASTGen_checkMacroDefinition( extern "C" ptrdiff_t swift_ASTGen_expandFreestandingMacro( void *diagEngine, void *macro, uint8_t externalKind, - const char *discriminator, ptrdiff_t discriminatorLength, void *sourceFile, + const char *discriminator, ptrdiff_t discriminatorLength, + uint8_t rawMacroRole, void *sourceFile, const void *sourceLocation, const char **evaluatedSource, ptrdiff_t *evaluatedSourceLength); @@ -901,6 +902,13 @@ evaluateFreestandingMacro(FreestandingMacroExpansion *expansion, #endif }); + // Only one freestanding macro role is permitted, so look at the roles to + // figure out which one to use. + MacroRole macroRole = + macroRoles.contains(MacroRole::Expression) ? MacroRole::Expression + : macroRoles.contains(MacroRole::Declaration) ? MacroRole::Declaration + : MacroRole::CodeItem; + auto macroDef = macro->getDefinition(); switch (macroDef.kind) { case MacroDefinition::Kind::Undefined: @@ -961,7 +969,8 @@ evaluateFreestandingMacro(FreestandingMacroExpansion *expansion, swift_ASTGen_expandFreestandingMacro( &ctx.Diags, externalDef->opaqueHandle, static_cast(externalDef->kind), discriminator->data(), - discriminator->size(), astGenSourceFile, + discriminator->size(), + static_cast(macroRole), astGenSourceFile, expansion->getSourceRange().Start.getOpaquePointerValue(), &evaluatedSourceAddress, &evaluatedSourceLength); if (!evaluatedSourceAddress) diff --git a/test/Macros/Inputs/syntax_macro_definitions.swift b/test/Macros/Inputs/syntax_macro_definitions.swift index 7eaa3505105c4..b01cb48da66dc 100644 --- a/test/Macros/Inputs/syntax_macro_definitions.swift +++ b/test/Macros/Inputs/syntax_macro_definitions.swift @@ -73,6 +73,26 @@ public struct StringifyMacro: ExpressionMacro { } } +public struct ExprAndDeclMacro: ExpressionMacro, DeclarationMacro { + public static func expansion( + of macro: some FreestandingMacroExpansionSyntax, + in context: some MacroExpansionContext + ) -> ExprSyntax { + guard let argument = macro.argumentList.first?.expression else { + fatalError("boom") + } + + return "(\(argument), \(StringLiteralExprSyntax(content: argument.description)))" + } + + public static func expansion( + of macro: some FreestandingMacroExpansionSyntax, + in context: some MacroExpansionContext + ) -> [DeclSyntax] { + return [] + } +} + public struct StringifyAndTryMacro: ExpressionMacro { public static func expansion( of macro: some FreestandingMacroExpansionSyntax, diff --git a/test/Macros/macro_expand.swift b/test/Macros/macro_expand.swift index 3d5fdb5e2fde2..6c842263a5f19 100644 --- a/test/Macros/macro_expand.swift +++ b/test/Macros/macro_expand.swift @@ -509,3 +509,27 @@ func testHasEqualsSelf( _ = (zP == true) _ = (wP == true) } + +// Macro whose implementation is both an expression and declaration macro. +@freestanding(declaration) +macro AsDeclMacro(_ value: T) = #externalMacro(module: "MacroDefinition", type: "ExprAndDeclMacro") + +@freestanding(expression) +macro AsExprMacro(_ value: T) -> (T, String) = #externalMacro(module: "MacroDefinition", type: "ExprAndDeclMacro") + +func testExpressionAndDeclarationMacro() { + #AsExprMacro(1 + 1) // expected-warning{{expression of type '(Int, String)' is unused}} + struct Inner { + #AsDeclMacro(1 + 1) + } + #AsDeclMacro(1 + 1) +} + +// Expression macro implementation with declaration macro role +@freestanding(declaration) macro stringifyAsDeclMacro(_ value: T) = #externalMacro(module: "MacroDefinition", type: "StringifyMacro") +func testExpressionAsDeclarationMacro() { +#if TEST_DIAGNOSTICS + #stringifyAsDeclMacro(1+1) + // expected-error@-1{{macro implementation type 'StringifyMacro' doesn't conform to required protocol 'DeclarationMacro' (from macro 'stringifyAsDeclMacro')}} +#endif +} diff --git a/test/Macros/macro_plugin_basic.swift b/test/Macros/macro_plugin_basic.swift index fbd9633f4095a..d62321f20dfab 100644 --- a/test/Macros/macro_plugin_basic.swift +++ b/test/Macros/macro_plugin_basic.swift @@ -23,9 +23,9 @@ // CHECK: ->(plugin:[[#PID:]]) {"getCapability":{}} // CHECK: <-(plugin:[[#PID]]) {"getCapabilityResult":{"capability":{"protocolVersion":1}}} -// CHECK: ->(plugin:[[#PID]]) {"expandFreestandingMacro":{"discriminator":"$s{{.+}}","macro":{"moduleName":"TestPlugin","name":"testString","typeName":"TestStringMacro"},"syntax":{"kind":"expression","location":{"column":19,"fileID":"MyApp/test.swift","fileName":"BUILD_DIR{{.+}}test.swift","line":5,"offset":301},"source":"#testString(123)"}}} +// CHECK: ->(plugin:[[#PID]]) {"expandFreestandingMacro":{"discriminator":"$s{{.+}}","macro":{"moduleName":"TestPlugin","name":"testString","typeName":"TestStringMacro"},"macroRole":"expression","syntax":{"kind":"expression","location":{"column":19,"fileID":"MyApp/test.swift","fileName":"BUILD_DIR{{.+}}test.swift","line":5,"offset":301},"source":"#testString(123)"}}} // CHECK: <-(plugin:[[#PID]]) {"expandFreestandingMacroResult":{"diagnostics":[],"expandedSource":"\"123\"\n + \"foo \""}} -// CHECK: ->(plugin:[[#PID]]) {"expandFreestandingMacro":{"discriminator":"$s{{.+}}","macro":{"moduleName":"TestPlugin","name":"testStringWithError","typeName":"TestStringWithErrorMacro"},"syntax":{"kind":"expression","location":{"column":19,"fileID":"MyApp/test.swift","fileName":"BUILD_DIR{{.+}}test.swift","line":6,"offset":336},"source":"#testStringWithError(321)"}}} +// CHECK: ->(plugin:[[#PID]]) {"expandFreestandingMacro":{"discriminator":"$s{{.+}}","macro":{"moduleName":"TestPlugin","name":"testStringWithError","typeName":"TestStringWithErrorMacro"},"macroRole":"expression","syntax":{"kind":"expression","location":{"column":19,"fileID":"MyApp/test.swift","fileName":"BUILD_DIR{{.+}}test.swift","line":6,"offset":336},"source":"#testStringWithError(321)"}}} // CHECK: <-(plugin:[[#PID]]) {"expandFreestandingMacroResult":{"diagnostics":[{"fixIts":[],"highlights":[],"message":"message from plugin","notes":[],"position":{"fileName":"BUILD_DIR{{.*}}test.swift","offset":336},"severity":"error"}],"expandedSource":"\"bar\""}} //--- test.swift diff --git a/test/Macros/macro_plugin_error.swift b/test/Macros/macro_plugin_error.swift index 458ec20891e0f..3f599142f7397 100644 --- a/test/Macros/macro_plugin_error.swift +++ b/test/Macros/macro_plugin_error.swift @@ -23,12 +23,12 @@ // CHECK: ->(plugin:[[#PID1:]]) {"getCapability":{}} // CHECK-NEXT: <-(plugin:[[#PID1]]) {"getCapabilityResult":{"capability":{"protocolVersion":1}}} -// CHECK-NEXT: ->(plugin:[[#PID1]]) {"expandFreestandingMacro":{"discriminator":"$s{{.+}}","macro":{"moduleName":"TestPlugin","name":"fooMacro","typeName":"FooMacro"},"syntax":{"kind":"expression","location":{"column":19,"fileID":"MyApp/test.swift","fileName":"BUILD_DIR{{.+}}test.swift","line":6,"offset":[[#]]},"source":"#fooMacro(1)"}}} +// CHECK-NEXT: ->(plugin:[[#PID1]]) {"expandFreestandingMacro":{"discriminator":"$s{{.+}}","macro":{"moduleName":"TestPlugin","name":"fooMacro","typeName":"FooMacro"},"macroRole":"expression","syntax":{"kind":"expression","location":{"column":19,"fileID":"MyApp/test.swift","fileName":"BUILD_DIR{{.+}}test.swift","line":6,"offset":[[#]]},"source":"#fooMacro(1)"}}} // CHECK-NEXT: <-(plugin:[[#PID1]]) {"invalidResponse":{}} -// CHECK-NEXT: ->(plugin:[[#PID1]]) {"expandFreestandingMacro":{"discriminator":"$s{{.+}}","macro":{"moduleName":"TestPlugin","name":"fooMacro","typeName":"FooMacro"},"syntax":{"kind":"expression","location":{"column":19,"fileID":"MyApp/test.swift","fileName":"BUILD_DIR{{.+}}test.swift","line":8,"offset":[[#]]},"source":"#fooMacro(2)"}}} +// CHECK-NEXT: ->(plugin:[[#PID1]]) {"expandFreestandingMacro":{"discriminator":"$s{{.+}}","macro":{"moduleName":"TestPlugin","name":"fooMacro","typeName":"FooMacro"},"macroRole":"expression","syntax":{"kind":"expression","location":{"column":19,"fileID":"MyApp/test.swift","fileName":"BUILD_DIR{{.+}}test.swift","line":8,"offset":[[#]]},"source":"#fooMacro(2)"}}} // ^ This messages causes the mock plugin exit because there's no matching expected message. -// CHECK: ->(plugin:[[#PID2:]]) {"expandFreestandingMacro":{"discriminator":"$s{{.+}}","macro":{"moduleName":"TestPlugin","name":"fooMacro","typeName":"FooMacro"},"syntax":{"kind":"expression","location":{"column":19,"fileID":"MyApp/test.swift","fileName":"BUILD_DIR{{.+}}test.swift","line":10,"offset":[[#]]},"source":"#fooMacro(3)"}}} +// CHECK: ->(plugin:[[#PID2:]]) {"expandFreestandingMacro":{"discriminator":"$s{{.+}}","macro":{"moduleName":"TestPlugin","name":"fooMacro","typeName":"FooMacro"},"macroRole":"expression","syntax":{"kind":"expression","location":{"column":19,"fileID":"MyApp/test.swift","fileName":"BUILD_DIR{{.+}}test.swift","line":10,"offset":[[#]]},"source":"#fooMacro(3)"}}} // CHECK-NEXT: <-(plugin:[[#PID2:]]) {"expandFreestandingMacroResult":{"diagnostics":[],"expandedSource":"3.description"}} //--- test.swift diff --git a/test/Macros/macro_plugin_server.swift b/test/Macros/macro_plugin_server.swift index b4bc369b6b39d..66116d2dfdecc 100644 --- a/test/Macros/macro_plugin_server.swift +++ b/test/Macros/macro_plugin_server.swift @@ -37,16 +37,16 @@ // CHECK-NEXT: <-(plugin:[[#PID1]]) {"loadPluginLibraryResult":{"diagnostics":[],"loaded":true}} // CHECK-NEXT: ->(plugin:[[#PID1]]) {"loadPluginLibrary":{"libraryPath":"BUILD_DIR{{.*}}plugins/libEvilMacros.dylib","moduleName":"EvilMacros"}} // CHECK-NEXT: <-(plugin:[[#PID1]]) {"loadPluginLibraryResult":{"diagnostics":[],"loaded":true}} -// CHECK-NEXT: ->(plugin:[[#PID1]]) {"expandFreestandingMacro":{"discriminator":"${{.*}}","macro":{"moduleName":"MacroDefinition","name":"stringify","typeName":"StringifyMacro"},"syntax":{"kind":"expression","location":{{{.+}}},"source":"#stringify(a + b)"}}} +// CHECK-NEXT: ->(plugin:[[#PID1]]) {"expandFreestandingMacro":{"discriminator":"${{.*}}","macro":{"moduleName":"MacroDefinition","name":"stringify","typeName":"StringifyMacro"},"macroRole":"expression","syntax":{"kind":"expression","location":{{{.+}}},"source":"#stringify(a + b)"}}} // CHECK-NEXT: <-(plugin:[[#PID1]]) {"expandFreestandingMacroResult":{"diagnostics":[],"expandedSource":"(a + b, \"a + b\")"}} -// CHECK-NEXT: ->(plugin:[[#PID1]]) {"expandFreestandingMacro":{"discriminator":"${{.*}}","macro":{"moduleName":"EvilMacros","name":"evil","typeName":"CrashingMacro"},"syntax":{"kind":"expression","location":{{{.+}}},"source":"#evil(42)"}}} +// CHECK-NEXT: ->(plugin:[[#PID1]]) {"expandFreestandingMacro":{"discriminator":"${{.*}}","macro":{"moduleName":"EvilMacros","name":"evil","typeName":"CrashingMacro"},"macroRole":"expression","syntax":{"kind":"expression","location":{{{.+}}},"source":"#evil(42)"}}} // ^ This crashes the plugin server. // CHECK-NEXT: ->(plugin:[[#PID2:]]) {"loadPluginLibrary":{"libraryPath":"BUILD_DIR{{.*}}plugins/libMacroDefinition.dylib","moduleName":"MacroDefinition"}} // CHECK-NEXT: <-(plugin:[[#PID2]]) {"loadPluginLibraryResult":{"diagnostics":[],"loaded":true}} // CHECK-NEXT: ->(plugin:[[#PID2]]) {"loadPluginLibrary":{"libraryPath":"BUILD_DIR{{.*}}plugins/libEvilMacros.dylib","moduleName":"EvilMacros"}} // CHECK-NEXT: <-(plugin:[[#PID2]]) {"loadPluginLibraryResult":{"diagnostics":[],"loaded":true}} -// CHECK-NEXT: ->(plugin:[[#PID2]]) {"expandFreestandingMacro":{"discriminator":"${{.*}}","macro":{"moduleName":"MacroDefinition","name":"stringify","typeName":"StringifyMacro"},"syntax":{"kind":"expression","location":{{{.+}}},"source":"#stringify(b + a)"}}} +// CHECK-NEXT: ->(plugin:[[#PID2]]) {"expandFreestandingMacro":{"discriminator":"${{.*}}","macro":{"moduleName":"MacroDefinition","name":"stringify","typeName":"StringifyMacro"},"macroRole":"expression","syntax":{"kind":"expression","location":{{{.+}}},"source":"#stringify(b + a)"}}} // CHECK-NEXT: <-(plugin:[[#PID2]]) {"expandFreestandingMacroResult":{"diagnostics":[],"expandedSource":"(b + a, \"b + a\")"}} @freestanding(expression) macro stringify(_ value: T) -> (T, String) = #externalMacro(module: "MacroDefinition", type: "StringifyMacro") From 8569bafe4744f3ecbc6aedf073ff0d27470a7df1 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 9 Jun 2023 13:40:11 -0700 Subject: [PATCH 3/6] Diagnose macros that have multiple freestanding macro roles. Per SE-0397, a macro may only have a single freestanding macro role, otherwise we would have an ambiguity in how a particular freestanding macro would be expanded. Produce an error on such macro declarations. Fixes rdar://110178899. (cherry picked from commit c5ec3892aacd6680db140ad969b7072bfcbf7c8a) --- include/swift/AST/DiagnosticsSema.def | 2 ++ lib/Sema/TypeCheckDeclPrimary.cpp | 18 ++++++++++++++++++ test/Macros/macros_diagnostics.swift | 7 +++++++ test/Macros/parsing.swift | 1 + 4 files changed, 28 insertions(+) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 35e16935d0f0f..b3dcedcb8ce1b 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -7060,6 +7060,8 @@ NOTE(macro_remove_result_type,none, ()) NOTE(macro_make_freestanding_expression,none, "make this macro a freestanding expression macro", ()) +ERROR(macro_multiple_freestanding_roles,none, + "macro can only have a single freestanding role", ()) ERROR(macro_expansion_missing_pound,none, "expansion of macro %0 requires leading '#'", (DeclName)) ERROR(macro_expansion_missing_arguments,none, diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp index 36f055daad036..0a6bdd8f2f35a 100644 --- a/lib/Sema/TypeCheckDeclPrimary.cpp +++ b/lib/Sema/TypeCheckDeclPrimary.cpp @@ -2018,6 +2018,17 @@ class DeclChecker : public DeclVisitor { llvm_unreachable("should always be type-checked already"); } + /// Determine the number of bits set. + static unsigned numBitsSet(uint64_t value) { + unsigned count = 0; + for (uint64_t i : range(0, 63)) { + if (value & (uint64_t(1) << i)) + ++count; + } + + return count; + } + void visitMacroDecl(MacroDecl *MD) { TypeChecker::checkDeclAttributes(MD); checkAccessControl(MD); @@ -2078,6 +2089,13 @@ class DeclChecker : public DeclVisitor { .fixItRemove(SourceRange(MD->arrowLoc, resultTypeRepr->getEndLoc())); } } + + // A macro can only have a single freestanding macro role. + MacroRoles freestandingRolesInhabited = + MD->getMacroRoles() & getFreestandingMacroRoles(); + if (numBitsSet(freestandingRolesInhabited.toRaw()) > 1) { + MD->diagnose(diag::macro_multiple_freestanding_roles); + } } void visitMacroExpansionDecl(MacroExpansionDecl *MED) { diff --git a/test/Macros/macros_diagnostics.swift b/test/Macros/macros_diagnostics.swift index 1955d9f6a76ec..2f9b00a44716e 100644 --- a/test/Macros/macros_diagnostics.swift +++ b/test/Macros/macros_diagnostics.swift @@ -211,3 +211,10 @@ struct SomeType { @freestanding(declaration) macro nonExpressionReturnsVoid(_: T) -> Void = #externalMacro(module: "A", type: "B") // expected-warning@-1{{external macro implementation type}} + + +@freestanding(expression) +@freestanding(declaration) +macro multipleFreestandingRoles(_: T) -> Void = #externalMacro(module: "A", type: "B") +// expected-warning@-1{{external macro implementation type}} +// expected-error@-2{{macro can only have a single freestanding role}} diff --git a/test/Macros/parsing.swift b/test/Macros/parsing.swift index 1de66078abec3..f4380d1ecfd24 100644 --- a/test/Macros/parsing.swift +++ b/test/Macros/parsing.swift @@ -35,6 +35,7 @@ protocol Q { associatedtype Assoc } @freestanding(expression) @freestanding(declaration, names: named(Foo)) @attached(accessor) macro m10(_: String) = #externalMacro(module: "A", type: "M4") // expected-warning@-1{{external macro implementation type 'A.M4' could not be found for macro 'm10'}} +// expected-error@-2{{macro can only have a single freestanding role}} @attached( accessor, From 5d54406a23c65a7bfe908c9cabc7b1f7a4bec56c Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 9 Jun 2023 21:51:26 -0700 Subject: [PATCH 4/6] [Macros] Tighten restriction on non-expression macros not having return types There is no reason to special-case `Void` to permit it. Rather, make this a syntactic rule. Thanks to Alex Hoppen for the suggestion. --- lib/Sema/TypeCheckDeclPrimary.cpp | 5 ++--- test/Macros/attached_macros_diags.swift | 6 +++--- test/Macros/macros_diagnostics.swift | 3 +++ test/Macros/parsing.swift | 2 +- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp index 0a6bdd8f2f35a..9c57355b5359a 100644 --- a/lib/Sema/TypeCheckDeclPrimary.cpp +++ b/lib/Sema/TypeCheckDeclPrimary.cpp @@ -2073,11 +2073,10 @@ class DeclChecker : public DeclVisitor { } } - // If the macro has a (non-Void) result type, it must have the freestanding + // If the macro has a result type, it must have the freestanding // expression role. Other roles cannot have result types. if (auto resultTypeRepr = MD->getResultTypeRepr()) { - if (!MD->getMacroRoles().contains(MacroRole::Expression) && - !MD->getResultInterfaceType()->isEqual(Ctx.getVoidType())) { + if (!MD->getMacroRoles().contains(MacroRole::Expression)) { auto resultType = MD->getResultInterfaceType(); Ctx.Diags.diagnose( MD->arrowLoc, diag::macro_result_type_cannot_be_used, resultType) diff --git a/test/Macros/attached_macros_diags.swift b/test/Macros/attached_macros_diags.swift index bc592ed27bd86..636d8deb07476 100644 --- a/test/Macros/attached_macros_diags.swift +++ b/test/Macros/attached_macros_diags.swift @@ -6,17 +6,17 @@ // expected-warning@-1{{external macro implementation type 'MyMacros.Macro1' could not be found for macro 'm1()'}} // expected-note@-2{{'m1()' declared here}} -@attached(accessor) macro m2(_: Int) -> Void = #externalMacro(module: "MyMacros", type: "Macro2") +@attached(accessor) macro m2(_: Int) = #externalMacro(module: "MyMacros", type: "Macro2") // expected-warning@-1{{external macro implementation type 'MyMacros.Macro2' could not be found for macro 'm2'}} // expected-note@-2{{candidate has partially matching parameter list (Int)}} // expected-note@-3{{candidate expects value of type 'Int' for parameter #1 (got 'String')}} -@attached(accessor) macro m2(_: Double) -> Void = #externalMacro(module: "MyMacros", type: "Macro2") +@attached(accessor) macro m2(_: Double) = #externalMacro(module: "MyMacros", type: "Macro2") // expected-warning@-1{{external macro implementation type 'MyMacros.Macro2' could not be found for macro 'm2'}} // expected-note@-2{{candidate has partially matching parameter list (Double)}} // expected-note@-3{{candidate expects value of type 'Double' for parameter #1 (got 'String')}} -@attached(accessor) macro m3(message: String) -> Void = #externalMacro(module: "MyMacros", type: "Macro3") +@attached(accessor) macro m3(message: String) = #externalMacro(module: "MyMacros", type: "Macro3") // expected-warning@-1{{external macro implementation type 'MyMacros.Macro3' could not be found for macro 'm3(message:)'}} @freestanding(expression) macro stringify(_ value: T) -> (T, String) = #externalMacro(module: "MyMacros", type: "StringifyMacro") diff --git a/test/Macros/macros_diagnostics.swift b/test/Macros/macros_diagnostics.swift index 2f9b00a44716e..21acc484a0b44 100644 --- a/test/Macros/macros_diagnostics.swift +++ b/test/Macros/macros_diagnostics.swift @@ -211,6 +211,9 @@ struct SomeType { @freestanding(declaration) macro nonExpressionReturnsVoid(_: T) -> Void = #externalMacro(module: "A", type: "B") // expected-warning@-1{{external macro implementation type}} +// expected-error@-2{{only a freestanding expression macro can produce a result of type 'Void'}} +// expected-note@-3{{make this macro a freestanding expression macro}}{{1-1=@freestanding(expression)\n}} +// expected-note@-4{{remove the result type if the macro does not produce a value}}{{68-76=}} @freestanding(expression) diff --git a/test/Macros/parsing.swift b/test/Macros/parsing.swift index f4380d1ecfd24..ffc039386d8d7 100644 --- a/test/Macros/parsing.swift +++ b/test/Macros/parsing.swift @@ -51,7 +51,7 @@ macro am1() named, // expected-error{{introduced name kind 'named' requires a single argument '(name)'}} arbitrary(a) // expected-error{{introduced name kind 'arbitrary' must not have an argument}} ) -macro am2() -> Void +macro am2() // expected-error@-1{{macro 'am2()' requires a definition}} #m1 + 1 From 1744b465dc35d78683305fe59fa3712a4fb7c0af Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 9 Jun 2023 23:16:57 -0700 Subject: [PATCH 5/6] [AST printer] Stop printing `-> ()` types on all macro declarations. --- lib/AST/ASTPrinter.cpp | 8 +++----- test/ModuleInterface/macros.swift | 6 +++--- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp index 85d43a6554fd1..64d3aa77ab4df 100644 --- a/lib/AST/ASTPrinter.cpp +++ b/lib/AST/ASTPrinter.cpp @@ -4780,16 +4780,14 @@ void PrintAST::visitMacroDecl(MacroDecl *decl) { } ); - { + if (decl->resultType.getTypeRepr() || + !decl->getResultInterfaceType()->isVoid()) { Printer.printStructurePre(PrintStructureKind::DeclResultTypeClause); SWIFT_DEFER { Printer.printStructurePost(PrintStructureKind::DeclResultTypeClause); }; - if (decl->parameterList) - Printer << " -> "; - else - Printer << ": "; + Printer << " -> "; TypeLoc resultTypeLoc( decl->resultType.getTypeRepr(), decl->getResultInterfaceType()); diff --git a/test/ModuleInterface/macros.swift b/test/ModuleInterface/macros.swift index 57af7ecbafbfa..63ea8332a7ad3 100644 --- a/test/ModuleInterface/macros.swift +++ b/test/ModuleInterface/macros.swift @@ -30,14 +30,14 @@ @freestanding(expression) public macro publicLine() -> T = #externalMacro(module: "SomeModule", type: "Line") // CHECK: #if compiler(>=5.3) && $Macros -// CHECK: @attached(accessor) public macro myWrapper() -> () = #externalMacro(module: "SomeModule", type: "Wrapper") +// CHECK: @attached(accessor) public macro myWrapper() = #externalMacro(module: "SomeModule", type: "Wrapper") // CHECK-NEXT: #endif @attached(accessor) public macro myWrapper() = #externalMacro(module: "SomeModule", type: "Wrapper") // CHECK: #if compiler(>=5.3) && $Macros && $AttachedMacros -// CHECK: @attached(member, names: named(`init`), prefixed(`$`)) public macro MemberwiseInit() -> () = #externalMacro(module: "SomeModule", type: "MemberwiseInitMacro") +// CHECK: @attached(member, names: named(`init`), prefixed(`$`)) public macro MemberwiseInit() = #externalMacro(module: "SomeModule", type: "MemberwiseInitMacro") // CHECK-NEXT: #endif -@attached(member, names: named(init), prefixed(`$`)) public macro MemberwiseInit() -> () = #externalMacro(module: "SomeModule", type: "MemberwiseInitMacro") +@attached(member, names: named(init), prefixed(`$`)) public macro MemberwiseInit() = #externalMacro(module: "SomeModule", type: "MemberwiseInitMacro") // CHECK-NOT: internalStringify @freestanding(expression) macro internalStringify(_ value: T) -> (T, String) = #externalMacro(module: "SomeModule", type: "StringifyMacro") From c72f379917f0fb7ee68d3eaded0532afc4f58f01 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 9 Jun 2023 23:22:09 -0700 Subject: [PATCH 6/6] [Macros] Downgrade "void return for non-expression macro" error in interfaces This allows us to continue to accept Swift interface files created with older versions of the Swift 5.9 compiler that emitted a spurious `-> ()` on non-expression macro declarations. --- lib/Sema/TypeCheckDeclPrimary.cpp | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp index 9c57355b5359a..3c06ccd47e0d8 100644 --- a/lib/Sema/TypeCheckDeclPrimary.cpp +++ b/lib/Sema/TypeCheckDeclPrimary.cpp @@ -2077,10 +2077,21 @@ class DeclChecker : public DeclVisitor { // expression role. Other roles cannot have result types. if (auto resultTypeRepr = MD->getResultTypeRepr()) { if (!MD->getMacroRoles().contains(MacroRole::Expression)) { - auto resultType = MD->getResultInterfaceType(); - Ctx.Diags.diagnose( - MD->arrowLoc, diag::macro_result_type_cannot_be_used, resultType) - .highlight(resultTypeRepr->getSourceRange()); + auto resultType = MD->getResultInterfaceType(); { + auto diag = Ctx.Diags.diagnose( + MD->arrowLoc, diag::macro_result_type_cannot_be_used, resultType); + diag.highlight(resultTypeRepr->getSourceRange()); + + // In a .swiftinterface file, downgrade this diagnostic to a warning. + // This allows the compiler to process existing .swiftinterface + // files that contain this issue. + if (resultType->isVoid()) { + if (auto sourceFile = MD->getParentSourceFile()) + if (sourceFile->Kind == SourceFileKind::Interface) + diag.limitBehavior(DiagnosticBehavior::Warning); + } + } + Ctx.Diags.diagnose(MD->arrowLoc, diag::macro_make_freestanding_expression) .fixItInsert(MD->getAttributeInsertionLoc(false), "@freestanding(expression)\n");