Skip to content

Commit 4c7e461

Browse files
committed
Improve captured field diagnostics
This patch improves the error message emitted when the capture list contains an item that is a sub-field of a struct/class/etc.... If the closure capture did not include `weak` at the beginning, the presence of a period would cause the if-chain to fall through the identifier checking, resulting in an error message about expecting a `weak` keyword. Instead, I've opted to accept the period at that stage of parsing so that we can fall through to a better error message. For the following code ``` { [self.field] in ... } ``` instead of emitting `expected 'weak', 'unowned', or no specifier in capture list`, we now emit `fields may only be captured by assigning to a specific name` with a fix-it that changes the code to ``` { [ field = self.field ] in ... } ```
1 parent 70044d7 commit 4c7e461

File tree

2 files changed

+42
-5
lines changed

2 files changed

+42
-5
lines changed

lib/Parse/ParseExpr.cpp

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2582,7 +2582,8 @@ ParserStatus Parser::parseClosureSignatureIfPresent(
25822582
diagnose(Tok, diag::attr_unowned_expected_rparen);
25832583
}
25842584
} else if (Tok.isAny(tok::identifier, tok::kw_self, tok::code_complete) &&
2585-
peekToken().isAny(tok::equal, tok::comma, tok::r_square)) {
2585+
peekToken().isAny(tok::equal, tok::comma, tok::r_square,
2586+
tok::period)) {
25862587
// "x = 42", "x," and "x]" are all strong captures of x.
25872588
} else {
25882589
diagnose(Tok, diag::expected_capture_specifier);
@@ -2626,8 +2627,20 @@ ParserStatus Parser::parseClosureSignatureIfPresent(
26262627
// It is a common error to try to capture a nested field instead of just
26272628
// a local name, reject it with a specific error message.
26282629
if (Tok.isAny(tok::period, tok::exclaim_postfix,tok::question_postfix)){
2629-
diagnose(Tok, diag::cannot_capture_fields);
2630-
skipUntil(tok::comma, tok::r_square);
2630+
auto diag = diagnose(Tok, diag::cannot_capture_fields);
2631+
while (peekToken().isNot(tok::comma, tok::r_square, tok::eof,
2632+
tok::kw_in, tok::r_brace,
2633+
tok::pound_endif, tok::pound_else,
2634+
tok::pound_elseif))
2635+
consumeToken();
2636+
if (Tok.isKeyword() || Tok.isContextualDeclKeyword()) {
2637+
StringRef name = Tok.getText();
2638+
diag.fixItInsert(nameLoc, ("`" + name + "` = ").str());
2639+
} else if (Tok.is(tok::identifier)) {
2640+
StringRef name = Tok.getRawText();
2641+
diag.fixItInsert(nameLoc, (name + " = ").str());
2642+
}
2643+
skipSingle(); // Advance to the comma or r_square
26312644
continue;
26322645
}
26332646

test/expr/closure/closures.swift

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,8 @@ enum ImplicitSelfAllowedInEnum {
301301

302302
class SomeClass {
303303
var field : SomeClass?
304+
var `class` : SomeClass?
305+
var `in`: Int = 0
304306
func foo() -> Int {}
305307
}
306308

@@ -341,8 +343,30 @@ extension SomeClass {
341343
doStuff { [weak xyz = self.field] in xyz!.foo() }
342344

343345
// rdar://16889886 - Assert when trying to weak capture a property of self in a lazy closure
344-
// FIXME: We should probably offer a fix-it to the field capture error and suppress the 'implicit self' error. https://bugs.swift.org/browse/SR-11634
345-
doStuff { [weak self.field] in field!.foo() } // expected-error {{fields may only be captured by assigning to a specific name}} expected-error {{reference to property 'field' in closure requires explicit use of 'self' to make capture semantics explicit}} expected-note {{reference 'self.' explicitly}} {{36-36=self.}} expected-note {{capture 'self' explicitly to enable implicit 'self' in this closure}} {{16-16=self, }}
346+
doStuff { [weak self.field] in field!.foo() }
347+
// expected-error@-1{{fields may only be captured by assigning to a specific name}}{{21-21=field = }}
348+
// expected-error@-2{{reference to property 'field' in closure requires explicit use of 'self' to make capture semantics explicit}}
349+
// expected-note@-3{{reference 'self.' explicitly}} {{36-36=self.}}
350+
// expected-note@-4{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{16-16=self, }}
351+
352+
doStuff { [self.field] in field!.foo() }
353+
// expected-error@-1{{fields may only be captured by assigning to a specific name}}{{16-16=field = }}
354+
// expected-error@-2{{reference to property 'field' in closure requires explicit use of 'self' to make capture semantics explicit}}
355+
// expected-note@-3{{reference 'self.' explicitly}} {{31-31=self.}}
356+
// expected-note@-4{{capture 'self' explicitly to enable implicit 'self' in this closure}} {{16-16=self, }}
357+
358+
doStuff { [self.field!.foo()] in 32 }
359+
//expected-error@-1{{fields may only be captured by assigning to a specific name}}
360+
361+
doStuff { [self.class] in self.class!.foo() }
362+
//expected-error@-1{{fields may only be captured by assigning to a specific name}}{{16-16=`class` = }}
363+
364+
doStuff { [self.`in`] in `in` }
365+
//expected-note@-1{{capture 'self' explicitly to enable implicit 'self' in this closure}}
366+
//expected-error@-2{{fields may only be captured by assigning to a specific name}}{{16-16=`in` = }}
367+
//expected-error@-3{{reference to property 'in' in closure requires explicit use of 'self' to make capture semantics explicit}}
368+
//expected-note@-4{{reference 'self.' explicitly}}
369+
346370
// expected-warning @+1 {{variable 'self' was written to, but never read}}
347371
doStuff { [weak self&field] in 42 } // expected-error {{expected ']' at end of capture list}}
348372

0 commit comments

Comments
 (0)