Skip to content
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.

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

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

Open
swift-ci opened this issue Feb 20, 2016 · 13 comments
Open

Comments

@swift-ci
Copy link
Contributor

swift-ci commented Feb 20, 2016

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
Copy link

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

@belkadan
Copy link

Looks like this warns as expected now.

@swift-ci
Copy link
Contributor Author

swift-ci commented Aug 9, 2016

Comment by Chris Eidhof (JIRA)

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

@belkadan
Copy link

belkadan commented Aug 9, 2016

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

@swift-ci
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link

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

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@AnthonyLatsis AnthonyLatsis added the good first issue Good for newcomers label Jul 6, 2022
@AnthonyLatsis
Copy link
Contributor

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.

@ahoppen ahoppen removed bug Something isn't working good first issue Good for newcomers labels Sep 14, 2023
@ahoppen ahoppen transferred this issue from apple/swift Sep 14, 2023
@ahoppen ahoppen transferred this issue from apple/swift-syntax Sep 14, 2023
@ahoppen ahoppen added bug Something isn't working good first issue Good for newcomers labels Sep 14, 2023
@AnthonyLatsis
Copy link
Contributor

@ahoppen Why did you transfer this back?

@ahoppen
Copy link
Collaborator

ahoppen commented Sep 14, 2023

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
Copy link
Contributor

AnthonyLatsis commented Sep 16, 2023

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
Copy link
Collaborator

ahoppen commented Sep 16, 2023

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 ahoppen removed bug Something isn't working good first issue Good for newcomers labels Sep 16, 2023
@ahoppen ahoppen transferred this issue from apple/swift Sep 16, 2023
@ahoppen
Copy link
Collaborator

ahoppen commented Sep 16, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants