Skip to content

Commit 2d2cd18

Browse files
committed
[CS] Avoid doing performStmtDiagnostics in CSApply
Instead, ensure we walk into expressions in SyntacticDiagnosticWalker, allowing `performStmtDiagnostics` to be called there for all statements present in the target. This avoids a case where we'd double diagnose, and avoids diagnosing macro arguments; instead we diagnose the expansions.
1 parent 72b8d22 commit 2d2cd18

File tree

4 files changed

+49
-20
lines changed

4 files changed

+49
-20
lines changed

lib/Sema/CSSyntacticElement.cpp

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1733,20 +1733,6 @@ class SyntacticElementSolutionApplication
17331733
return Type();
17341734
}
17351735

1736-
ASTNode visit(Stmt *S, bool performSyntacticDiagnostics = true) {
1737-
auto rewritten = ASTVisitor::visit(S);
1738-
if (!rewritten)
1739-
return {};
1740-
1741-
if (performSyntacticDiagnostics) {
1742-
if (auto *stmt = getAsStmt(rewritten)) {
1743-
performStmtDiagnostics(stmt, context.getAsDeclContext());
1744-
}
1745-
}
1746-
1747-
return rewritten;
1748-
}
1749-
17501736
bool visitPatternBindingDecl(PatternBindingDecl *PBD) {
17511737
// If this is a placeholder variable with an initializer, we just need to
17521738
// set the inferred type.
@@ -2274,7 +2260,7 @@ class ResultBuilderRewriter : public SyntacticElementSolutionApplication {
22742260
private:
22752261
ASTNode visitDoStmt(DoStmt *doStmt) override {
22762262
if (auto transformed = transformDo(doStmt)) {
2277-
return visit(transformed.get(), /*performSyntacticDiagnostics=*/false);
2263+
return visit(transformed.get());
22782264
}
22792265

22802266
auto newBody = visit(doStmt->getBody());

lib/Sema/TypeCheckConstraints.cpp

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,7 @@ namespace {
320320
class SyntacticDiagnosticWalker final : public ASTWalker {
321321
const SyntacticElementTarget &Target;
322322
bool IsTopLevelExprStmt;
323+
unsigned ExprDepth = 0;
323324

324325
SyntacticDiagnosticWalker(const SyntacticElementTarget &target,
325326
bool isExprStmt)
@@ -335,10 +336,23 @@ class SyntacticDiagnosticWalker final : public ASTWalker {
335336
return MacroWalking::Expansion;
336337
}
337338

338-
PreWalkResult<Expr *> walkToExprPre(Expr *expr) override {
339-
auto isExprStmt = (expr == Target.getAsExpr()) ? IsTopLevelExprStmt : false;
340-
performSyntacticExprDiagnostics(expr, Target.getDeclContext(), isExprStmt);
341-
return Action::SkipNode(expr);
339+
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
340+
// We only want to call the diagnostic logic for the top-level expression,
341+
// since the underlying logic will visit each sub-expression. We want to
342+
// continue walking however to diagnose any child statements in e.g
343+
// closures.
344+
if (ExprDepth == 0) {
345+
auto isExprStmt = (E == Target.getAsExpr()) ? IsTopLevelExprStmt : false;
346+
performSyntacticExprDiagnostics(E, Target.getDeclContext(), isExprStmt);
347+
}
348+
ExprDepth += 1;
349+
return Action::Continue(E);
350+
}
351+
352+
PostWalkResult<Expr *> walkToExprPost(Expr *E) override {
353+
assert(ExprDepth > 0);
354+
ExprDepth -= 1;
355+
return Action::Continue(E);
342356
}
343357

344358
PreWalkResult<Stmt *> walkToStmtPre(Stmt *stmt) override {

test/Constraints/result_builder_diags.swift

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1029,3 +1029,31 @@ func testMissingElementInEmptyBuilder() {
10291029
func test2() -> Int {}
10301030
// expected-error@-1 {{expected expression of type 'Int' in result builder 'SingleElementBuilder'}} {{24-24=<#T##Int#>}}
10311031
}
1032+
1033+
// https://github.com/swiftlang/swift/issues/77453
1034+
func testNoDuplicateStmtDiags() {
1035+
@resultBuilder
1036+
struct Builder {
1037+
static func buildBlock<T>(_ components: T...) -> T {
1038+
components.first!
1039+
}
1040+
static func buildEither<T>(first component: T) -> T {
1041+
component
1042+
}
1043+
static func buildEither<T>(second component: T) -> T {
1044+
component
1045+
}
1046+
}
1047+
1048+
func takesClosure(_ fn: () -> Void) -> Int? { nil }
1049+
1050+
@Builder
1051+
func foo() -> Int {
1052+
if let x = takesClosure {} {
1053+
// expected-warning@-1 {{trailing closure in this context is confusable with the body of the statement}}
1054+
x
1055+
} else {
1056+
1
1057+
}
1058+
}
1059+
}

test/Macros/top_level_freestanding.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,9 @@ func testGlobalVariable() {
112112

113113
// expected-note @+1 2 {{in expansion of macro 'anonymousTypes' here}}
114114
#anonymousTypes { () -> String in
115-
// expected-error @+1 {{use of protocol 'Equatable' as a type must be written 'any Equatable'}}
116115
_ = 0 as Equatable
116+
// DIAG_BUFFERS-DAG: @__swiftmacro_9MacroUser0033top_level_freestandingswift_DbGHjfMX[[@LINE-3]]{{.*}}anonymousTypesfMf1_.swift:4:16: error: use of protocol 'Equatable' as a type must be written 'any Equatable'
117+
117118
return "foo"
118119
}
119120

0 commit comments

Comments
 (0)