Skip to content

Commit bb58cbc

Browse files
hamishknighthamishknight
committed
[Sema] Avoid double-diagnosing in macro expansions
Avoid walking into macro expansions for MiscDiagnostics, the expansions will instead be visited when they're type-checked on their own.
1 parent 72b8d22 commit bb58cbc

File tree

4 files changed

+138
-17
lines changed

4 files changed

+138
-17
lines changed

‎lib/Sema/MiscDiagnostics.cpp

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,6 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC,
9797
DiagnoseWalker(const DeclContext *DC, bool isExprStmt)
9898
: IsExprStmt(isExprStmt), Ctx(DC->getASTContext()), DC(DC) {}
9999

100-
MacroWalking getMacroWalkingBehavior() const override {
101-
return MacroWalking::Expansion;
102-
}
103-
104100
PreWalkAction walkToTypeReprPre(TypeRepr *T) override {
105101
return Action::Continue();
106102
}
@@ -1581,7 +1577,9 @@ static void diagRecursivePropertyAccess(const Expr *E, const DeclContext *DC) {
15811577
}
15821578

15831579
MacroWalking getMacroWalkingBehavior() const override {
1584-
return MacroWalking::Expansion;
1580+
// Macro expansions will be walked when they're type-checked, not as
1581+
// part of the surrounding code.
1582+
return MacroWalking::None;
15851583
}
15861584

15871585
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
@@ -4576,7 +4574,9 @@ static void checkStmtConditionTrailingClosure(ASTContext &ctx, const Expr *E) {
45764574
DiagnoseWalker(ASTContext &ctx) : Ctx(ctx) { }
45774575

45784576
MacroWalking getMacroWalkingBehavior() const override {
4579-
return MacroWalking::Expansion;
4577+
// Macro expansions will be walked when they're type-checked, not as
4578+
// part of the surrounding code.
4579+
return MacroWalking::None;
45804580
}
45814581

45824582
PreWalkResult<ArgumentList *>
@@ -4699,7 +4699,9 @@ class ObjCSelectorWalker : public ASTWalker {
46994699
: Ctx(dc->getASTContext()), DC(dc), SelectorTy(selectorTy) { }
47004700

47014701
MacroWalking getMacroWalkingBehavior() const override {
4702-
return MacroWalking::Expansion;
4702+
// Macro expansions will be walked when they're type-checked, not as
4703+
// part of the surrounding code.
4704+
return MacroWalking::None;
47034705
}
47044706

47054707
PreWalkResult<Expr *> walkToExprPre(Expr *expr) override {
@@ -5561,7 +5563,9 @@ static void diagnoseUnintendedOptionalBehavior(const Expr *E,
55615563
}
55625564

55635565
MacroWalking getMacroWalkingBehavior() const override {
5564-
return MacroWalking::Expansion;
5566+
// Macro expansions will be walked when they're type-checked, not as
5567+
// part of the surrounding code.
5568+
return MacroWalking::None;
55655569
}
55665570

55675571
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
@@ -5634,7 +5638,9 @@ static void diagnoseDeprecatedWritableKeyPath(const Expr *E,
56345638
}
56355639

56365640
MacroWalking getMacroWalkingBehavior() const override {
5637-
return MacroWalking::Expansion;
5641+
// Macro expansions will be walked when they're type-checked, not as
5642+
// part of the surrounding code.
5643+
return MacroWalking::None;
56385644
}
56395645

56405646
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
@@ -5719,7 +5725,9 @@ static void maybeDiagnoseCallToKeyValueObserveMethod(const Expr *E,
57195725
}
57205726

57215727
MacroWalking getMacroWalkingBehavior() const override {
5722-
return MacroWalking::Expansion;
5728+
// Macro expansions will be walked when they're type-checked, not as
5729+
// part of the surrounding code.
5730+
return MacroWalking::None;
57235731
}
57245732

57255733
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
@@ -5770,7 +5778,9 @@ static void diagnoseExplicitUseOfLazyVariableStorage(const Expr *E,
57705778
}
57715779

57725780
MacroWalking getMacroWalkingBehavior() const override {
5773-
return MacroWalking::Expansion;
5781+
// Macro expansions will be walked when they're type-checked, not as
5782+
// part of the surrounding code.
5783+
return MacroWalking::None;
57745784
}
57755785

57765786
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
@@ -5901,7 +5911,9 @@ static void diagnoseComparisonWithNaN(const Expr *E, const DeclContext *DC) {
59015911
}
59025912

59035913
MacroWalking getMacroWalkingBehavior() const override {
5904-
return MacroWalking::Expansion;
5914+
// Macro expansions will be walked when they're type-checked, not as
5915+
// part of the surrounding code.
5916+
return MacroWalking::None;
59055917
}
59065918

59075919
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
@@ -5934,7 +5946,9 @@ static void diagUnqualifiedAccessToMethodNamedSelf(const Expr *E,
59345946
DiagnoseWalker(const DeclContext *DC) : Ctx(DC->getASTContext()), DC(DC) {}
59355947

59365948
MacroWalking getMacroWalkingBehavior() const override {
5937-
return MacroWalking::Expansion;
5949+
// Macro expansions will be walked when they're type-checked, not as
5950+
// part of the surrounding code.
5951+
return MacroWalking::None;
59385952
}
59395953

59405954
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
@@ -6089,7 +6103,9 @@ diagnoseDictionaryLiteralDuplicateKeyEntries(const Expr *E,
60896103
DiagnoseWalker(const DeclContext *DC) : Ctx(DC->getASTContext()) {}
60906104

60916105
MacroWalking getMacroWalkingBehavior() const override {
6092-
return MacroWalking::Expansion;
6106+
// Macro expansions will be walked when they're type-checked, not as
6107+
// part of the surrounding code.
6108+
return MacroWalking::None;
60936109
}
60946110

60956111
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {

‎lib/Sema/MiscDiagnostics.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,10 @@ namespace swift {
138138
return Action::VisitNodeIf(isa<PatternBindingDecl>(D));
139139
}
140140

141-
// Only emit diagnostics in the expansion of macros.
142141
MacroWalking getMacroWalkingBehavior() const override {
143-
return MacroWalking::Expansion;
142+
// Macro expansions will be walked when they're type-checked, not as
143+
// part of the surrounding code.
144+
return MacroWalking::None;
144145
}
145146
};
146147

‎lib/Sema/TypeCheckConstraints.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,9 @@ class SyntacticDiagnosticWalker final : public ASTWalker {
332332
}
333333

334334
MacroWalking getMacroWalkingBehavior() const override {
335-
return MacroWalking::Expansion;
335+
// Macro expansions will be walked when they're type-checked, not as
336+
// part of the surrounding code.
337+
return MacroWalking::None;
336338
}
337339

338340
PreWalkResult<Expr *> walkToExprPre(Expr *expr) override {

‎test/Macros/macro_misc_diags.swift

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// REQUIRES: swift_swift_parser
2+
3+
// RUN: %empty-directory(%t)
4+
// RUN: split-file --leading-lines %s %t
5+
6+
// 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
7+
8+
// 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
9+
// RUN: %FileCheck --check-prefix=CHECK-DIAG --implicit-check-not="{{error|warning}}: " -input-file=%t/diags %s
10+
11+
//--- MacroPlugin.swift
12+
import SwiftSyntax
13+
import SwiftSyntaxMacros
14+
15+
public struct IdentityMacro: ExpressionMacro {
16+
public static func expansion(
17+
of macro: some FreestandingMacroExpansionSyntax,
18+
in context: some MacroExpansionContext
19+
) -> ExprSyntax {
20+
guard let argument = macro.arguments.first else {
21+
fatalError()
22+
}
23+
return "\(argument)"
24+
}
25+
}
26+
27+
public struct TrailingClosureMacro: ExpressionMacro {
28+
public static func expansion(
29+
of macro: some FreestandingMacroExpansionSyntax,
30+
in context: some MacroExpansionContext
31+
) -> ExprSyntax {
32+
guard let argument = macro.trailingClosure else {
33+
fatalError()
34+
}
35+
return "\(argument)"
36+
}
37+
}
38+
39+
public struct MakeBinding : DeclarationMacro {
40+
static public func expansion(
41+
of node: some FreestandingMacroExpansionSyntax,
42+
in context: some MacroExpansionContext
43+
) throws -> [DeclSyntax] {
44+
guard let arg = node.arguments.first else {
45+
fatalError()
46+
}
47+
return ["let x = \(arg)"]
48+
}
49+
}
50+
51+
//--- Client.swift
52+
@freestanding(expression)
53+
macro identity<T>(_ x: T) -> T = #externalMacro(module: "MacroPlugin", type: "IdentityMacro")
54+
55+
@freestanding(expression)
56+
macro trailingClosure<T>(_ x: T) -> T = #externalMacro(module: "MacroPlugin", type: "TrailingClosureMacro")
57+
58+
@freestanding(declaration, names: named(x))
59+
macro makeBinding<T>(_ x: T) = #externalMacro(module: "MacroPlugin", type: "MakeBinding")
60+
61+
@available(*, deprecated)
62+
func deprecatedFunc() -> Int { 0 }
63+
64+
_ = #identity(Int)
65+
// CHECK-DIAG: @__swiftmacro_6Client0017Clientswift_yEEFcfMX[[@LINE-2]]{{.*}}identityfMf_.swift:1:1: error: expected member name or initializer call after type name
66+
67+
_ = {
68+
_ = #identity(Int)
69+
// CHECK-DIAG: @__swiftmacro_6Client0017Clientswift_yEEFcfMX[[@LINE-2]]{{.*}}identityfMf0_.swift:1:1: error: expected member name or initializer call after type name
70+
}
71+
72+
// Currently we also do availability diagnostics for macro arguments.
73+
// FIXME: Should we only do this for expansions?
74+
_ = #identity(deprecatedFunc())
75+
// CHECK-DIAG: @__swiftmacro_6Client0017Clientswift_yEEFcfMX[[@LINE-2]]{{.*}}identityfMf1_.swift:1:1: warning: 'deprecatedFunc()' is deprecated
76+
// CHECK-DIAG: Client.swift:[[@LINE-2]]:15: warning: 'deprecatedFunc()' is deprecated
77+
78+
#makeBinding(deprecatedFunc())
79+
// CHECK-DIAG: Client.swift:[[@LINE-1]]:14: warning: 'deprecatedFunc()' is deprecated
80+
// CHECK-DIAG: @__swiftmacro_6Client0017Clientswift_yEEFcfMX[[@LINE-3]]{{.*}}makeBindingfMf_.swift:1:9: warning: 'deprecatedFunc()' is deprecated
81+
// CHECK-DIAG: @__swiftmacro_6Client0017Clientswift_yEEFcfMX[[@LINE-4]]{{.*}}makeBindingfMf_.swift:1:5: warning: initialization of immutable value 'x' was never used
82+
83+
struct S1 {
84+
#makeBinding(deprecatedFunc())
85+
// CHECK-DIAG: Client.swift:[[@LINE-1]]:16: warning: 'deprecatedFunc()' is deprecated
86+
// CHECK-DIAG: @__swiftmacro_6Client0017Clientswift_yEEFcfMX[[@LINE-3]]{{.*}}makeBindingfMf_.swift:1:9: warning: 'deprecatedFunc()' is deprecated
87+
}
88+
89+
struct S2 {
90+
#makeBinding({deprecatedFunc()})
91+
// CHECK-DIAG: Client.swift:[[@LINE-1]]:17: warning: 'deprecatedFunc()' is deprecated
92+
// CHECK-DIAG: @__swiftmacro_6Client0017Clientswift_yEEFcfMX[[@LINE-3]]{{.*}}makeBindingfMf_.swift:2:5: warning: 'deprecatedFunc()' is deprecated
93+
}
94+
95+
func takesClosure(_ fn: () -> Void) -> Int? { nil }
96+
97+
// FIXME: We should only be diagnosing the expansion here.
98+
_ = #trailingClosure {
99+
if let _ = takesClosure {} {}
100+
// 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
101+
// 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
102+
}

0 commit comments

Comments
 (0)