From b83cf1eaa5339381b508ff457cf0af65e63a53f0 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Mon, 13 Oct 2025 13:59:25 +0100 Subject: [PATCH] [Parse] Avoid setting closure `in` SourceLoc if missing Clients should be able to handle closures without a valid `in` SourceLoc, let's avoid setting it to a recovery location. This avoids crashing in cases where we set an `in` loc that's after the closure's end loc. --- lib/Parse/ParseExpr.cpp | 1 - lib/Sema/TypeCheckMacros.cpp | 14 +++++++------- .../Inputs/syntax_macro_definitions.swift | 18 ++++++++++++++++++ test/Macros/macro_expand_body_closure.swift | 17 +++++++++++++++++ .../IDE/crashers_fixed/pr-84278.swift | 2 ++ .../3ba9659c560ebf6a.swift | 2 +- 6 files changed, 45 insertions(+), 9 deletions(-) create mode 100644 test/Macros/macro_expand_body_closure.swift create mode 100644 validation-test/IDE/crashers_fixed/pr-84278.swift rename validation-test/{compiler_crashers_2 => compiler_crashers_2_fixed}/3ba9659c560ebf6a.swift (86%) diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp index 167d4e7b8bab0..b0ebea4757324 100644 --- a/lib/Parse/ParseExpr.cpp +++ b/lib/Parse/ParseExpr.cpp @@ -2897,7 +2897,6 @@ ParserStatus Parser::parseClosureSignatureIfPresent( diagnose(Tok, diag::expected_closure_in) .fixItInsert(Tok.getLoc(), "in "); } - inLoc = Tok.getLoc(); } } diff --git a/lib/Sema/TypeCheckMacros.cpp b/lib/Sema/TypeCheckMacros.cpp index d15c34929807b..76fce9780d894 100644 --- a/lib/Sema/TypeCheckMacros.cpp +++ b/lib/Sema/TypeCheckMacros.cpp @@ -977,14 +977,14 @@ static CharSourceRange getExpansionInsertionRange(MacroRole role, if (auto *expr = target.dyn_cast()) { ASSERT(isa(expr)); - auto *closure = cast(expr); // A closure body macro expansion replaces the full source - // range of the closure body starting from `in` and ending right - // before the closing brace. - return Lexer::getCharSourceRangeFromSourceRange( - sourceMgr, SourceRange(Lexer::getLocForEndOfToken( - sourceMgr, closure->getInLoc()), - closure->getEndLoc())); + // range of the closure body's contents. + auto *closure = cast(expr); + if (auto range = closure->getBody()->getContentRange()) + return Lexer::getCharSourceRangeFromSourceRange(sourceMgr, range); + + // If we have an empty body, just use the end loc. + return CharSourceRange(closure->getEndLoc(), 0); } // If the function has a body, that's what's being replaced. diff --git a/test/Macros/Inputs/syntax_macro_definitions.swift b/test/Macros/Inputs/syntax_macro_definitions.swift index bdc0716a8e4f9..4085204e49624 100644 --- a/test/Macros/Inputs/syntax_macro_definitions.swift +++ b/test/Macros/Inputs/syntax_macro_definitions.swift @@ -2725,6 +2725,24 @@ struct ThrowCancellationMacro: BodyMacro { } } +struct EmptyBodyMacro: BodyMacro { + static func expansion( + of node: AttributeSyntax, + providingBodyFor declaration: some DeclSyntaxProtocol & WithOptionalCodeBlockSyntax, + in context: some MacroExpansionContext + ) throws -> [CodeBlockItemSyntax] { + [] + } + + static func expansion( + of node: AttributeSyntax, + providingBodyFor closure: ClosureExprSyntax, + in context: some MacroExpansionContext + ) throws -> [CodeBlockItemSyntax] { + [] + } +} + @_spi(ExperimentalLanguageFeature) public struct TracedPreambleMacro: PreambleMacro { public static func expansion( diff --git a/test/Macros/macro_expand_body_closure.swift b/test/Macros/macro_expand_body_closure.swift new file mode 100644 index 0000000000000..337df03c1b8a5 --- /dev/null +++ b/test/Macros/macro_expand_body_closure.swift @@ -0,0 +1,17 @@ +// REQUIRES: swift_swift_parser +// REQUIRES: swift_feature_ClosureBodyMacro + +// RUN: %empty-directory(%t) +// RUN: %host-build-swift -swift-version 5 -emit-library -o %t/%target-library-name(MacroDefinition) -module-name=MacroDefinition %S/Inputs/syntax_macro_definitions.swift -g -no-toolchain-stdlib-rpath -swift-version 5 + +// RUN: %target-typecheck-verify-swift -swift-version 5 -load-plugin-library %t/%target-library-name(MacroDefinition) -module-name MacroUser -enable-experimental-feature ClosureBodyMacro + +@attached(body) +macro Empty() = #externalMacro(module: "MacroDefinition", type: "EmptyBodyMacro") + +// Make sure we can handle the lack of 'in' here. +_ = { @Empty x -> // expected-error {{cannot infer type of closure parameter 'x' without a type annotation}} + 0 // expected-error {{expected closure result type after '->'}} expected-error {{expected 'in' after the closure signature}} +} + +_ = { @Empty in } diff --git a/validation-test/IDE/crashers_fixed/pr-84278.swift b/validation-test/IDE/crashers_fixed/pr-84278.swift new file mode 100644 index 0000000000000..096d152c97806 --- /dev/null +++ b/validation-test/IDE/crashers_fixed/pr-84278.swift @@ -0,0 +1,2 @@ +// RUN: %target-swift-ide-test -code-completion -batch-code-completion -skip-filecheck -code-completion-diagnostics -source-filename %s +{ k -> #^^# diff --git a/validation-test/compiler_crashers_2/3ba9659c560ebf6a.swift b/validation-test/compiler_crashers_2_fixed/3ba9659c560ebf6a.swift similarity index 86% rename from validation-test/compiler_crashers_2/3ba9659c560ebf6a.swift rename to validation-test/compiler_crashers_2_fixed/3ba9659c560ebf6a.swift index 9cb0d2c3dfeb1..d17c3d386c939 100644 --- a/validation-test/compiler_crashers_2/3ba9659c560ebf6a.swift +++ b/validation-test/compiler_crashers_2_fixed/3ba9659c560ebf6a.swift @@ -1,4 +1,4 @@ // {"kind":"typecheck","signature":"swift::ast_scope::ASTScopeImpl::checkSourceRangeBeforeAddingChild(swift::ast_scope::ASTScopeImpl*, swift::ASTContext const&) const","signatureAssert":"Assertion failed: ((SM.isBefore(range.Start, range.End) || range.Start == range.End) && \"scope source range ends before start\"), function getCharSourceRangeOfScope"} -// RUN: not --crash %target-swift-frontend -typecheck %s +// RUN: not %target-swift-frontend -typecheck %s { @in