Skip to content

[Macros] Tighten restriction on non-expression macros not having return types #66526

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

Merged
merged 3 commits into from
Jun 10, 2023
Merged

[Macros] Tighten restriction on non-expression macros not having return types #66526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c78afdc
[Macros] Tighten restriction on non-expression macros not having retu…
DougGregor Jun 10, 2023
7fb1ba9
[AST printer] Stop printing `-> ()` types on all macro declarations.
DougGregor Jun 10, 2023
f7de3a3
[Macros] Downgrade "void return for non-expression macro" error in in…
DougGregor Jun 10, 2023
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
8 changes: 3 additions & 5 deletions lib/AST/ASTPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4811,16 +4811,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());
Expand Down
24 changes: 17 additions & 7 deletions lib/Sema/TypeCheckDeclPrimary.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2072,15 +2072,25 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
}
}

// 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())) {
auto resultType = MD->getResultInterfaceType();
Ctx.Diags.diagnose(
MD->arrowLoc, diag::macro_result_type_cannot_be_used, resultType)
.highlight(resultTypeRepr->getSourceRange());
if (!MD->getMacroRoles().contains(MacroRole::Expression)) {
auto resultType = MD->getResultInterfaceType(); {
auto diag = Ctx.Diags.diagnose(
Comment on lines +2079 to +2080
Copy link
Member

Choose a reason for hiding this comment

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

This looks really odd to me. Why do you open a new brace scope at the same line as

auto resultType = MD->getResultInterfaceType();

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");
Expand Down
6 changes: 3 additions & 3 deletions test/Macros/attached_macros_diags.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>(_ value: T) -> (T, String) = #externalMacro(module: "MyMacros", type: "StringifyMacro")
Expand Down
3 changes: 3 additions & 0 deletions test/Macros/macros_diagnostics.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,9 @@ struct SomeType {

@freestanding(declaration) macro nonExpressionReturnsVoid<T>(_: 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)
Expand Down
2 changes: 1 addition & 1 deletion test/Macros/parsing.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions test/ModuleInterface/macros.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@
@freestanding(expression) public macro publicLine<T: ExpressibleByIntegerLiteral>() -> 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<T>(_ value: T) -> (T, String) = #externalMacro(module: "SomeModule", type: "StringifyMacro")