[SR-778] Forgetting the in keyword can sometimes lead to compiling code

[swift-ci]

Previous ID	SR-778
Radar	None
Original Reporter	chriseidhof (JIRA User)
Type	Bug
Status	Reopened
Resolution

Additional Detail from JIRA

| | | |------------------|-----------------| |Votes | 0 | |Component/s | Standard Library | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: 0093a5f05b0f3ded251f13fdd054fe04

relates to:

• SR-245 Missing @warn_unused_result attributes in Standard Library

Issue Description:

Try the following code:

var x = 0
let capture: () -> Int = { [x]
  return x
}
x++
capture()

Expected result: because there is a capture list [x] that should capture x by value, we expect the result of the final call to be 0. However, it is 1. This is because we forget the in keyword, and [x] gets parsed as an array literal.

A possible solution would be to warn about [x] being an unused expression.

[belkadan]

Yeah, I'm just going to say this is a stdlib issue to add @warn_unused_result.

[belkadan]

Looks like this warns as expected now.

[swift-ci]

Comment by Chris Eidhof (JIRA)

This is actually still broken (DEVELOPMENT-SNAPSHOT-2016-08-04-a).

[belkadan]

But you get a warning about it. What more were you expecting?

[swift-ci]

Comment by Chris Eidhof (JIRA)

I'm not getting a warning... (The only thing that's broken in my example is the ++ operator which needs to be +=1).

[swift-ci]

Comment by Chris Eidhof (JIRA)

Interesting, I only tested it in a playground.

Upon further investigation: I now do get a warning, but only when I run it from the command-line. In an Xcode playground, I don't get a warning at all.

[belkadan]

Oh, right, because playgrounds actually do things with results, so they're not just discarded. Hm.

[AnthonyLatsis]

Would be nice to have a special warning for when the opening brace is followed on the same line by something that successfully parses as a capture list, and the alleged capture list is followed by neither in nor a type. Sounds like a decent starter bug.

[AnthonyLatsis]

@ahoppen Why did you transfer this back?

[ahoppen]

Because the code itself is valid as far is the parser is concerned. You can totally have an array literal on the first line of the closure. I think the type checker already warns that the result of the array is unused and we should probably improve the diagnostic in the type checker to suggest inserting in, instead of warning about Expression of type '[Int]' is unused.

[AnthonyLatsis]

No it sure is valid code, and we do emit a generic warning in the type checker. But I think this deserves a specialized, actionable warning, and that this warning is best implemented in the parser, where we have the necessary tools to check for line breaks and whether something parses as a capture list. Another benefit of a parser warning is that we can extrapolate it to variations where the capture list is unambiguous.

let capture: () -> Int = { [weak x]
  return x
}

[ahoppen]

Oh, I see what you mean now. We should unconditionally warn in the parser if the following conditions are satisfied

• An array literal on the same line that contains the closure’s {.
• The array literal is followed by a newline
• The closure contains multiple code block items

To implement this, we need to add infrastructure in the parser to emit warnings (we currently only have infrastructure to emit lexer warnings). My best idea of how to design this, is that the parser sets the hasWarning flag on the array literal and then the parse diagnostic generator then emits the actual warning diagnostic. We would just need to figure out what to do if a syntax tree has hasWarning set but the diagnostic generator doesn’t emit a warning diagnostic. But maybe the solution here would be to emit a top-level warning saying that we found a compiler bug and asking for a bug report with the source file.

[ahoppen]

Tracked in Apple’s issue tracker as rdar://115599038