[Parse] Reject trailing closures on literals

[jtbandes]

1 { } was parsed as a call expression with a trailing closure. This made the diagnostics for var x = 1 { get { ... } } extremely bad. I believe this should never be allowed for literals, and refusing to parse it as a trailing closure allows much better diagnostics to kick in:

Before

error: use of unresolved identifier 'get'
var x: Int? = nil { get { return 2 } }
                    ^~~

After

error: variable with getter/setter cannot have an initial value
var x: Int? = nil { get { return 2 } }
    ^         ^~~

Resolves SR-3671, at least for cases where the initial value is a literal.

[jtbandes]

@swift-ci Please smoke test

[jtbandes]

@swift-ci please smoke test

[jtbandes]

@shahmishal the OS X build seems very unhealthy..possibly just temporary network issues. https://ci.swift.org/job/swift-PR-osx-smoke-test/4813/console

[jrose-apple]

Makes sense to me, and generally looks like an improvement in most cases.

[jrose-apple]

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

[jtbandes]

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.

[jrose-apple]

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

[jrose-apple]

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

[jtbandes]

@swift-ci please smoke test macOS platform

[rintaro]

This looks good to me.
"Only for literals" looks like a good idea.

We can never have the perfect (source compatible) solution for this problem, anyway.

func get(_: () ->  Int) {}
func foo(_: () -> Void) -> Int { return 1 }
var foo: Int = 1

let x: Int = foo {
    get { return 1 }
}

[jtbandes]

I suppose we could delay all of these diagnostics to Sema, where it can know for certain if the braced block is a valid closure or more like a getter/setter block. But it's still kind of a guess anyway.

[jrose-apple]

This is all on a recovery path, and this still seems like an incremental improvement. I say go for it.

[jtbandes]

Shall we wait for @DougGregor (who is the CODE_OWNER of Parse and Sema)? I'm not sure to what extent the code owners desire to comment on every little change that goes in their component.

[jrose-apple]

I think it's okay. The process is supposed to be that the code owner makes sure everything in their component gets reviewed by someone they trust, not that it's been reviewed by them specifically. And they can always comment post-merge, and as code owner have the power to revert if they disagree. Between me and @rintaro I think you've gotten sufficient review.

[jtbandes]

Thanks!

[DougGregor]

Code owners don't have to weigh in on or review everything... we just try to make sure nothing goes unreviewed.