Skip to content

[Parse] Reject trailing closures on literals #7202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 2, 2017
Merged

[Parse] Reject trailing closures on literals #7202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion lib/Parse/ParseExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1591,8 +1591,14 @@ ParserResult<Expr> Parser::parseExprPostfix(Diag<> ID, bool isExprBasic) {
if (Tok.is(tok::l_brace) && isValidTrailingClosure(isExprBasic, *this)) {
// FIXME: if Result has a trailing closure, break out.

// Stop after literal expressions, which may never have trailing closures.
const auto *callee = Result.get();
if (isa<LiteralExpr>(callee) || isa<CollectionExpr>(callee) ||
isa<TupleExpr>(callee))
break;

ParserResult<Expr> closure =
parseTrailingClosure(Result.get()->getSourceRange());
parseTrailingClosure(callee->getSourceRange());
if (closure.isNull()) return nullptr;

// Trailing closure implicitly forms a call.
Expand Down
8 changes: 5 additions & 3 deletions test/Constraints/closures.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,19 +64,21 @@ func test13811882() {


// <rdar://problem/21544303> QoI: "Unexpected trailing closure" should have a fixit to insert a 'do' statement
// <https://bugs.swift.org/browse/SR-3671>
func r21544303() {
var inSubcall = true
let val = true
var inSubcall = val
{ // expected-error {{expected 'do' keyword to designate a block of statements}} {{3-3=do }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems suspicious. What happens if you don't change it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The presence of the literal true triggered my new break; and so the {} was parsed as part of the variable declaration, leading to computed property must have accessors specified.

The logic that produces expected 'do' keyword to designate a block of statements only looks for a trailing closure, so no longer applies with literals after my change. This didn't seem like a huge problem, but definitely an area for possible improvement.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see, it's specifically because it's a variable. I missed that. All right, seems fine for now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do still want to see what @rintaro thinks; they've spent much more time than me in Parse recently.

}
inSubcall = false

// This is a problem, but isn't clear what was intended.
var somethingElse = true { // expected-error {{cannot call value of non-function type 'Bool'}}
var somethingElse = val { // expected-error {{cannot call value of non-function type 'Bool'}}
}
inSubcall = false

var v2 : Bool = false
v2 = true
v2 = val
{ // expected-error {{expected 'do' keyword to designate a block of statements}}
}
}
Expand Down
6 changes: 3 additions & 3 deletions test/Parse/recovery.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func missingControllingExprInIf() {
if { true } { // expected-error {{missing condition in an 'if' statement}} expected-error {{braced block of statements is an unused closure}} expected-error{{expression resolves to an unused function}} expected-error{{consecutive statements on a line must be separated by ';'}} {{14-14=;}} expected-warning {{boolean literal is unused}}
}

if { true }() { // expected-error {{missing condition in an 'if' statement}} expected-error{{consecutive statements on a line must be separated by ';'}} {{14-14=;}} expected-error{{cannot call value of non-function type '()'}} expected-warning {{boolean literal is unused}}
if { true }() { // expected-error {{missing condition in an 'if' statement}} expected-error {{braced block of statements is an unused closure}} expected-error 2 {{consecutive statements on a line must be separated by ';'}} expected-error {{expression resolves to an unused function}} expected-warning {{boolean literal is unused}}
}

// <rdar://problem/18940198>
Expand All @@ -113,7 +113,7 @@ func missingControllingExprInWhile() {
while { true } { // expected-error {{missing condition in a 'while' statement}} expected-error {{braced block of statements is an unused closure}} expected-error{{expression resolves to an unused function}} expected-error{{consecutive statements on a line must be separated by ';'}} {{17-17=;}} expected-warning {{boolean literal is unused}}
}

while { true }() { // expected-error {{missing condition in a 'while' statement}} expected-error{{consecutive statements on a line must be separated by ';'}} {{17-17=;}} expected-error{{cannot call value of non-function type '()'}} expected-warning {{boolean literal is unused}}
while { true }() { // expected-error {{missing condition in a 'while' statement}} expected-error {{braced block of statements is an unused closure}} expected-error 2 {{consecutive statements on a line must be separated by ';'}} expected-error {{expression resolves to an unused function}} expected-warning {{boolean literal is unused}}
}

// <rdar://problem/18940198>
Expand Down Expand Up @@ -223,7 +223,7 @@ func missingControllingExprInSwitch() {
case _: return // expected-error{{'case' label can only appear inside a 'switch' statement}}
}

switch { 42 }() { // expected-error {{expected expression in 'switch' statement}} expected-error {{all statements inside a switch must be covered by a 'case' or 'default'}} expected-error {{consecutive statements on a line must be separated by ';'}} {{16-16=;}} expected-error {{cannot call value of non-function type '()'}}
switch { 42 }() { // expected-error {{expected expression in 'switch' statement}} expected-error {{all statements inside a switch must be covered by a 'case' or 'default'}} expected-error {{braced block of statements is an unused closure}} expected-error 2 {{consecutive statements on a line must be separated by ';'}} expected-error {{expression resolves to an unused function}}
case _: return // expected-error{{'case' label can only appear inside a 'switch' statement}}
}
}
Expand Down
4 changes: 2 additions & 2 deletions test/Sema/immutability.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ let bad_property_1: Int { // expected-error {{'let' declarations cannot be co
return 42
}
}
let bad_property_2: Int = 0 {
get { // expected-error {{use of unresolved identifier 'get'}}
let bad_property_2: Int = 0 { // expected-error {{'let' declarations cannot be computed properties}} expected-error {{variable with getter/setter cannot have an initial value}}
get {
return 42
}
}
Expand Down
19 changes: 9 additions & 10 deletions test/decl/var/properties.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,20 +120,19 @@ var x15: Int {

// Disambiguated as stored property with a trailing closure in the initializer.
//
// FIXME: QoI could be much better here.
var disambiguateGetSet1a: Int = 0 {
get {} // expected-error {{use of unresolved identifier 'get'}}
var disambiguateGetSet1a: Int = 0 { // expected-error {{variable with getter/setter cannot have an initial value}}
get {}
}
var disambiguateGetSet1b: Int = 0 {
get { // expected-error {{use of unresolved identifier 'get'}}
var disambiguateGetSet1b: Int = 0 { // expected-error {{variable with getter/setter cannot have an initial value}}
get {
return 42
}
}
var disambiguateGetSet1c: Int = 0 {
set {} // expected-error {{use of unresolved identifier 'set'}}
var disambiguateGetSet1c: Int = 0 { // expected-error {{variable with getter/setter cannot have an initial value}}
set {} // expected-error {{variable with a setter must also have a getter}}
}
var disambiguateGetSet1d: Int = 0 {
set(newValue) {} // expected-error {{use of unresolved identifier 'set'}} expected-error {{use of unresolved identifier 'newValue'}}
var disambiguateGetSet1d: Int = 0 { // expected-error {{variable with getter/setter cannot have an initial value}}
set(newValue) {} // expected-error {{variable with a setter must also have a getter}}
}

// Disambiguated as stored property with a trailing closure in the initializer.
Expand Down Expand Up @@ -199,7 +198,7 @@ func disambiguateGetSet4Attr() {
}

// Disambiguated as stored property with a trailing closure in the initializer.
var disambiguateImplicitGet1: Int = 0 { // expected-error {{cannot call value of non-function type 'Int'}}
var disambiguateImplicitGet1: Int = 0 { // expected-error {{variable with getter/setter cannot have an initial value}}
return 42
}
var disambiguateImplicitGet2: Int = takeIntTrailingClosure {
Expand Down
24 changes: 24 additions & 0 deletions test/expr/closure/trailing.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,30 @@ func notPostfix() {
_ = 1 + takeFunc { $0 }
}

func notLiterals() {
struct SR3671 { // <https://bugs.swift.org/browse/SR-3671>
var v: Int = 1 { // expected-error {{variable with getter/setter cannot have an initial value}}
get {
return self.v
}
}
}

var x: Int? = nil { get { } } // expected-error {{variable with getter/setter cannot have an initial value}}
_ = 1 {}
// expected-error@-1 {{consecutive statements on a line must be separated by ';'}}
// expected-error@-2 {{braced block of statements is an unused closure}} expected-error@-2 {{expression resolves to an unused function}}
_ = "hello" {}
// expected-error@-1 {{consecutive statements on a line must be separated by ';'}}
// expected-error@-2 {{braced block of statements is an unused closure}} expected-error@-2 {{expression resolves to an unused function}}
_ = [42] {}
// expected-error@-1 {{consecutive statements on a line must be separated by ';'}}
// expected-error@-2 {{braced block of statements is an unused closure}} expected-error@-2 {{expression resolves to an unused function}}
_ = (6765, 10946, 17711) {}
// expected-error@-1 {{consecutive statements on a line must be separated by ';'}}
// expected-error@-2 {{braced block of statements is an unused closure}} expected-error@-2 {{expression resolves to an unused function}}
}

class C {
func map(_ x: (Int) -> Int) -> C { return self }
func filter(_ x: (Int) -> Bool) -> C { return self }
Expand Down