diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 86a6e4c5539e8..c08fa8aad9d98 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -408,6 +408,10 @@ namespace { /// Rewrites an expression by applying the solution of a constraint /// system to that expression. class ExprRewriter : public ExprVisitor { + // Delayed items to type-check. + SmallVector LocalDeclsToTypeCheck; + SmallVector MacrosToExpand; + public: ConstraintSystem &cs; DeclContext *dc; @@ -415,6 +419,23 @@ namespace { std::optional target; bool SuppressDiagnostics; + ExprRewriter(ConstraintSystem &cs, Solution &solution, + std::optional target, + bool suppressDiagnostics) + : cs(cs), dc(target ? target->getDeclContext() : cs.DC), + solution(solution), target(target), + SuppressDiagnostics(suppressDiagnostics) {} + + ConstraintSystem &getConstraintSystem() const { return cs; } + + void addLocalDeclToTypeCheck(Decl *D) { + LocalDeclsToTypeCheck.push_back(D); + } + + void addMacroToExpand(MacroExpansionExpr *E) { + MacrosToExpand.push_back(E); + } + /// Coerce the given tuple to another tuple type. /// /// \param expr The expression we're converting. @@ -2631,15 +2652,6 @@ namespace { } public: - ExprRewriter(ConstraintSystem &cs, Solution &solution, - std::optional target, - bool suppressDiagnostics) - : cs(cs), dc(target ? target->getDeclContext() : cs.DC), - solution(solution), target(target), - SuppressDiagnostics(suppressDiagnostics) {} - - ConstraintSystem &getConstraintSystem() const { return cs; } - /// Simplify the expression type and return the expression. /// /// This routine is used for 'simple' expressions that only need their @@ -5528,16 +5540,19 @@ namespace { // FIXME: Expansion should be lazy. // i.e. 'ExpandMacroExpansionExprRequest' should be sinked into - // 'getRewritten()', and performed on-demand. + // 'getRewritten()', and performed on-demand. Unfortunately that requires + // auditing every ASTWalker's `getMacroWalkingBehavior` since + // `MacroWalking::Expansion` does not actually kick expansion. if (!cs.Options.contains(ConstraintSystemFlags::DisableMacroExpansions) && // Do not expand macros inside macro arguments. For example for // '#stringify(#assert(foo))' when typechecking `#assert(foo)`, // we don't want to expand it. llvm::none_of(llvm::ArrayRef(ExprStack).drop_back(1), [](Expr *E) { return isa(E); })) { - (void)evaluateOrDefault(cs.getASTContext().evaluator, - ExpandMacroExpansionExprRequest{E}, - std::nullopt); + // We need to delay the expansion until we're done applying the solution + // since running MiscDiagnostics on the expansion may walk up and query + // the type of a parent closure, e.g `diagnoseImplicitSelfUseInClosure`. + addMacroToExpand(E); } cs.cacheExprTypes(E); @@ -5604,6 +5619,18 @@ namespace { diag::add_consume_to_silence) .fixItInsert(coercion->getStartLoc(), "consume "); } + + // Type-check any local decls encountered. + for (auto *D : LocalDeclsToTypeCheck) + TypeChecker::typeCheckDecl(D); + + // Expand any macros encountered. + // FIXME: Expansion should be lazy. + auto &eval = cs.getASTContext().evaluator; + for (auto *E : MacrosToExpand) { + (void)evaluateOrDefault(eval, ExpandMacroExpansionExprRequest{E}, + std::nullopt); + } } /// Diagnose an optional injection that is probably not what the @@ -8805,22 +8832,15 @@ namespace { class ExprWalker : public ASTWalker, public SyntacticElementTargetRewriter { ExprRewriter &Rewriter; - SmallVector LocalDeclsToTypeCheck; public: ExprWalker(ExprRewriter &Rewriter) : Rewriter(Rewriter) { } - ~ExprWalker() { - // Type-check any local decls encountered. - for (auto *D : LocalDeclsToTypeCheck) - TypeChecker::typeCheckDecl(D); - } - Solution &getSolution() const override { return Rewriter.solution; } DeclContext *&getCurrentDC() const override { return Rewriter.dc; } void addLocalDeclToTypeCheck(Decl *D) override { - LocalDeclsToTypeCheck.push_back(D); + Rewriter.addLocalDeclToTypeCheck(D); } bool shouldWalkIntoPropertyWrapperPlaceholderValue() override { diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp index 7c52a8d9403cb..d2484ea684de3 100644 --- a/lib/Sema/MiscDiagnostics.cpp +++ b/lib/Sema/MiscDiagnostics.cpp @@ -97,10 +97,6 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC, DiagnoseWalker(const DeclContext *DC, bool isExprStmt) : IsExprStmt(isExprStmt), Ctx(DC->getASTContext()), DC(DC) {} - MacroWalking getMacroWalkingBehavior() const override { - return MacroWalking::Expansion; - } - PreWalkAction walkToTypeReprPre(TypeRepr *T) override { return Action::Continue(); } @@ -1581,7 +1577,9 @@ static void diagRecursivePropertyAccess(const Expr *E, const DeclContext *DC) { } MacroWalking getMacroWalkingBehavior() const override { - return MacroWalking::Expansion; + // Macro expansions will be walked when they're type-checked, not as + // part of the surrounding code. + return MacroWalking::None; } PreWalkResult walkToExprPre(Expr *E) override { @@ -4576,7 +4574,9 @@ static void checkStmtConditionTrailingClosure(ASTContext &ctx, const Expr *E) { DiagnoseWalker(ASTContext &ctx) : Ctx(ctx) { } MacroWalking getMacroWalkingBehavior() const override { - return MacroWalking::Expansion; + // Macro expansions will be walked when they're type-checked, not as + // part of the surrounding code. + return MacroWalking::None; } PreWalkResult @@ -4699,7 +4699,9 @@ class ObjCSelectorWalker : public ASTWalker { : Ctx(dc->getASTContext()), DC(dc), SelectorTy(selectorTy) { } MacroWalking getMacroWalkingBehavior() const override { - return MacroWalking::Expansion; + // Macro expansions will be walked when they're type-checked, not as + // part of the surrounding code. + return MacroWalking::None; } PreWalkResult walkToExprPre(Expr *expr) override { @@ -5561,7 +5563,9 @@ static void diagnoseUnintendedOptionalBehavior(const Expr *E, } MacroWalking getMacroWalkingBehavior() const override { - return MacroWalking::Expansion; + // Macro expansions will be walked when they're type-checked, not as + // part of the surrounding code. + return MacroWalking::None; } PreWalkResult walkToExprPre(Expr *E) override { @@ -5634,7 +5638,9 @@ static void diagnoseDeprecatedWritableKeyPath(const Expr *E, } MacroWalking getMacroWalkingBehavior() const override { - return MacroWalking::Expansion; + // Macro expansions will be walked when they're type-checked, not as + // part of the surrounding code. + return MacroWalking::None; } PreWalkResult walkToExprPre(Expr *E) override { @@ -5719,7 +5725,9 @@ static void maybeDiagnoseCallToKeyValueObserveMethod(const Expr *E, } MacroWalking getMacroWalkingBehavior() const override { - return MacroWalking::Expansion; + // Macro expansions will be walked when they're type-checked, not as + // part of the surrounding code. + return MacroWalking::None; } PreWalkResult walkToExprPre(Expr *E) override { @@ -5770,7 +5778,9 @@ static void diagnoseExplicitUseOfLazyVariableStorage(const Expr *E, } MacroWalking getMacroWalkingBehavior() const override { - return MacroWalking::Expansion; + // Macro expansions will be walked when they're type-checked, not as + // part of the surrounding code. + return MacroWalking::None; } PreWalkResult walkToExprPre(Expr *E) override { @@ -5901,7 +5911,9 @@ static void diagnoseComparisonWithNaN(const Expr *E, const DeclContext *DC) { } MacroWalking getMacroWalkingBehavior() const override { - return MacroWalking::Expansion; + // Macro expansions will be walked when they're type-checked, not as + // part of the surrounding code. + return MacroWalking::None; } PreWalkResult walkToExprPre(Expr *E) override { @@ -5934,7 +5946,9 @@ static void diagUnqualifiedAccessToMethodNamedSelf(const Expr *E, DiagnoseWalker(const DeclContext *DC) : Ctx(DC->getASTContext()), DC(DC) {} MacroWalking getMacroWalkingBehavior() const override { - return MacroWalking::Expansion; + // Macro expansions will be walked when they're type-checked, not as + // part of the surrounding code. + return MacroWalking::None; } PreWalkResult walkToExprPre(Expr *E) override { @@ -6089,7 +6103,9 @@ diagnoseDictionaryLiteralDuplicateKeyEntries(const Expr *E, DiagnoseWalker(const DeclContext *DC) : Ctx(DC->getASTContext()) {} MacroWalking getMacroWalkingBehavior() const override { - return MacroWalking::Expansion; + // Macro expansions will be walked when they're type-checked, not as + // part of the surrounding code. + return MacroWalking::None; } PreWalkResult walkToExprPre(Expr *E) override { diff --git a/lib/Sema/MiscDiagnostics.h b/lib/Sema/MiscDiagnostics.h index deecbab75422b..6f784ff0abae8 100644 --- a/lib/Sema/MiscDiagnostics.h +++ b/lib/Sema/MiscDiagnostics.h @@ -138,9 +138,10 @@ namespace swift { return Action::VisitNodeIf(isa(D)); } - // Only emit diagnostics in the expansion of macros. MacroWalking getMacroWalkingBehavior() const override { - return MacroWalking::Expansion; + // Macro expansions will be walked when they're type-checked, not as + // part of the surrounding code. + return MacroWalking::None; } }; diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index f2524ef717f08..9ce41d049896d 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -332,7 +332,9 @@ class SyntacticDiagnosticWalker final : public ASTWalker { } MacroWalking getMacroWalkingBehavior() const override { - return MacroWalking::Expansion; + // Macro expansions will be walked when they're type-checked, not as + // part of the surrounding code. + return MacroWalking::None; } PreWalkResult walkToExprPre(Expr *expr) override { diff --git a/test/Macros/macro_misc_diags.swift b/test/Macros/macro_misc_diags.swift new file mode 100644 index 0000000000000..d7207429eb450 --- /dev/null +++ b/test/Macros/macro_misc_diags.swift @@ -0,0 +1,125 @@ +// REQUIRES: swift_swift_parser + +// RUN: %empty-directory(%t) +// RUN: split-file --leading-lines %s %t + +// RUN: %host-build-swift -swift-version 5 -emit-library -o %t/%target-library-name(MacroPlugin) -module-name=MacroPlugin %t/MacroPlugin.swift -g -no-toolchain-stdlib-rpath + +// RUN: not %target-swift-frontend -typecheck -swift-version 6 -load-plugin-library %t/%target-library-name(MacroPlugin) %t/Client.swift -module-name Client -diagnostic-style=llvm 2> %t/diags +// RUN: %FileCheck --check-prefix=CHECK-DIAG --implicit-check-not="{{error|warning}}: " -input-file=%t/diags %s + +//--- MacroPlugin.swift +import SwiftSyntax +import SwiftSyntaxMacros + +public struct IdentityMacro: ExpressionMacro { + public static func expansion( + of macro: some FreestandingMacroExpansionSyntax, + in context: some MacroExpansionContext + ) -> ExprSyntax { + guard let argument = macro.arguments.first else { + fatalError() + } + return "\(argument)" + } +} + +public struct TrailingClosureMacro: ExpressionMacro { + public static func expansion( + of macro: some FreestandingMacroExpansionSyntax, + in context: some MacroExpansionContext + ) -> ExprSyntax { + guard let argument = macro.trailingClosure else { + fatalError() + } + return "\(argument)" + } +} + +public struct MakeBinding : DeclarationMacro { + static public func expansion( + of node: some FreestandingMacroExpansionSyntax, + in context: some MacroExpansionContext + ) throws -> [DeclSyntax] { + guard let arg = node.arguments.first else { + fatalError() + } + return ["let x = \(arg)"] + } +} + +//--- Client.swift +@freestanding(expression) +macro identity(_ x: T) -> T = #externalMacro(module: "MacroPlugin", type: "IdentityMacro") + +@freestanding(expression) +macro trailingClosure(_ x: T) -> T = #externalMacro(module: "MacroPlugin", type: "TrailingClosureMacro") + +@freestanding(declaration, names: named(x)) +macro makeBinding(_ x: T) = #externalMacro(module: "MacroPlugin", type: "MakeBinding") + +@available(*, deprecated) +func deprecatedFunc() -> Int { 0 } + +// FIXME: We also ought to be diagnosing the macro argument +_ = #identity(Int) +// CHECK-DIAG: @__swiftmacro_6Client0017Clientswift_yEEFcfMX[[@LINE-2]]{{.*}}identityfMf_.swift:1:1: error: expected member name or initializer call after type name + +_ = { + _ = #identity(Int) + // CHECK-DIAG: @__swiftmacro_6Client0017Clientswift_yEEFcfMX[[@LINE-2]]{{.*}}identityfMf0_.swift:1:1: error: expected member name or initializer call after type name +} + +_ = #identity(deprecatedFunc()) +// CHECK-DIAG: @__swiftmacro_6Client0017Clientswift_yEEFcfMX[[@LINE-2]]{{.*}}identityfMf1_.swift:1:1: warning: 'deprecatedFunc()' is deprecated +// CHECK-DIAG: Client.swift:[[@LINE-2]]:15: warning: 'deprecatedFunc()' is deprecated + +#makeBinding(deprecatedFunc()) +// CHECK-DIAG: Client.swift:[[@LINE-1]]:14: warning: 'deprecatedFunc()' is deprecated +// CHECK-DIAG: @__swiftmacro_6Client0017Clientswift_yEEFcfMX[[@LINE-3]]{{.*}}makeBindingfMf_.swift:1:9: warning: 'deprecatedFunc()' is deprecated +// CHECK-DIAG: @__swiftmacro_6Client0017Clientswift_yEEFcfMX[[@LINE-4]]{{.*}}makeBindingfMf_.swift:1:5: warning: initialization of immutable value 'x' was never used + +struct S1 { + #makeBinding(deprecatedFunc()) + // CHECK-DIAG: Client.swift:[[@LINE-1]]:16: warning: 'deprecatedFunc()' is deprecated + // CHECK-DIAG: @__swiftmacro_6Client0017Clientswift_yEEFcfMX[[@LINE-3]]{{.*}}makeBindingfMf_.swift:1:9: warning: 'deprecatedFunc()' is deprecated +} + +struct S2 { + #makeBinding({deprecatedFunc()}) + // CHECK-DIAG: Client.swift:[[@LINE-1]]:17: warning: 'deprecatedFunc()' is deprecated + // CHECK-DIAG: @__swiftmacro_6Client0017Clientswift_yEEFcfMX[[@LINE-3]]{{.*}}makeBindingfMf_.swift:2:5: warning: 'deprecatedFunc()' is deprecated +} + +func takesClosure(_ fn: () -> Void) -> Int? { nil } + +_ = #trailingClosure { + if let _ = takesClosure {} {} + // CHECK-DIAG: Client.swift:[[@LINE-1]]:27: warning: trailing closure in this context is confusable with the body of the statement; pass as a parenthesized argument to silence this warning + // CHECK-DIAG: @__swiftmacro_6Client0017Clientswift_yEEFcfMX[[@LINE-4]]{{.*}}trailingClosurefMf_.swift:2:27: warning: trailing closure in this context is confusable with the body of the statement +} + +// rdar://138997009 - Make sure we don't crash in MiscDiagnostics' implicit +// self diagnosis. +struct rdar138997009 { + func foo() {} + func bar() { + _ = { + _ = #trailingClosure { + foo() + } + } + } +} + +class rdar138997009_Class { + func foo() {} + func bar() { + _ = { + _ = #trailingClosure { + foo() + // CHECK-DIAG: @__swiftmacro_6Client0017Clientswift_yEEFcfMX[[@LINE-3]]{{.*}}trailingClosurefMf_.swift:2:9: error: call to method 'foo' in closure requires explicit use of 'self' to make capture semantics explicit + } + } + } +}