[Parse][SR-510] emit error for closures outside of a func

[jder]

When there is no local context, we should emit an error rather than
silently eating the closure. This code used to be silent to avoid
double-reporting an error for fixed-size array types, but those
no longer exist.

Resolves https://bugs.swift.org/browse/SR-510

[dduan]

Passes build-script -RT.

(except for the existing failing test on master at the moment, not related to this PR.)

[jder]

Thanks, @dduan!

[jrose-apple]

The error message isn't really correct for a user:

• Accessors, initializers, and deinitializers are not considered "functions". (Nor exactly are closures.)
• I think you mean "initial value expression" instead of "initializer", since an "initializer" is "the thing you declare with init" in Swift.
• Script-mode files allow top-level code, including closures.

Stepping back, both of the other test changes aren't really adding any value. Admittedly those are just a mess of tokens and therefore a mess of error messages, but a user who ends up in that situation is probably not thinking of those braced-blocks as closures.

Why isn't this being caught by the normal enum case expression validation already?

[dduan]

@jrose-apple For enum rawValue, the enum expression parser simply gives Parser::parseExpr an diagnoses and defer the error handling to the latter. Which is why, I believe, @jder handled it here (where parseExpr eventually calls).

[dduan]

@jrose-apple for comparison, I attempted to catch the error at the enum error but ultimately decided solution here is better. #931

Part of this bug is when rawValue parsing fails, the case expression code simply quit silently. That's probably worth addressing too.

[jder]

@jrose-apple Thanks for the feedback. I agree that the error message is not great, but had missed some of your points.

I believe the way the current logic for parsing the raw values works is that parseExpr will always print an error message when it fails, falling back to the generic "expected raw case value" error if it is completely confused. If parseExpr succeeds parsing an expression, then the case parseDeclEnumCase validates that it is a literal, etc (which I think you're referring to about the case validation). In this case the expression was failing to parse at all, but parseExpr was not producing an error.

So, it seems like we could:

• Improve the error message.
• Fall back to the generic "expected raw case value" error rather than producing a specific diagnostic for this case. This would mean having parseExpr pass the "fallback" diagnostic along to parseExprClosure.
• Allow parseExprClosure to succeed without a local context (returning an ErrorExpr?) and let it get caught by the parseDeclEnumCase checks.

I personally think that the second option is probably the most reasonable, but it is currently unprecedented (the "fallback" diagnostic is only emitted in a single place right now) which is why I didn't do it in the first place. What do you think?

[jrose-apple]

Hm. What about just making a local context? I haven't looked at this code recently so I don't know how hard that is, but it seems like just parsing case values the same way as property initial values would be reasonable.

[jder]

Sure, I can look into that.

[jder]

@dduan If you'd like to take a look at that yourself, feel free to grab the bug from me.