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-4637] Add fix-it for removing '@escaping' from optional closure parameter #47214

Closed
swift-ci opened this issue Apr 20, 2017 · 14 comments
Closed
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself good first issue Good for newcomers

Comments

@swift-ci
Copy link
Collaborator

Previous ID SR-4637
Radar None
Original Reporter garricn (JIRA User)
Type Bug
Status Resolved
Resolution Done

Attachment: Download

Environment

Xcode 8.3.1, Swift 3.1

Additional Detail from JIRA
Votes 2
Component/s Compiler
Labels Bug, StarterBug
Assignee ismetanin (JIRA)
Priority Medium

md5: ea3ce9103e3b95d06baa97059732e723

Issue Description:

As of SR-2444, optional closures as function parameters are already escaping escaping by default. Currently, when adding the @escaping attribute to an optional closure parameter, the compiler throws the following error:

@escaping attribute only applies to function types

Since this type is already escaping escaping by default, it would be helpful to add / amend the error as follows:

Optional closures are already @escaping.

And add the following Fix-it:

Fix-it Delete "@escaping"
@swift-ci
Copy link
Collaborator Author

Comment by Garric Nahapetian (JIRA)

Thank you @belkadan for updating this SR. It's my fist so I'm just getting the hang of things. Any feedback/help is greatly appreciated 🙂

@milseman
Copy link
Mannequin

milseman mannequin commented Apr 20, 2017

I'm not sure "by default" is the correct phrasing here, as I don't think there's any way to make them non-escaping. "Optional closures are already escaping" could be a different error message, or alternatively a note. But the fixit would be very helpful!

garricn (JIRA User) ask if you have any questions

@swift-ci
Copy link
Collaborator Author

Comment by Garric Nahapetian (JIRA)

I certainly have/will have questions.

1) I plan to start working on it this weekend. I don't need to feel rushed, correct?
2) To be sure, am I to replace the error "@escaping attribute only applies to function types" with the error "Optional closures are already @escaping"? Or can / should I supplement the existing error with the new error containing the Fix-it?
3) I plan to do some logical investigating to try to find out how to solve this on my own. I keep hearing Ayaka's voice in my head "What else does this same or similar thing? Nevertheless, help is greatly appreciated!
4) I found this file: https://github.com/apple/swift/blob/30dd1a90474032717332f26aca8acbfda427d8bb/test/attr/attr_escaping.swift which looks like it has a lot to do with this except that its a test file, I think, and its a Swift file. I have a feeling I need to get into a .cpp file.
5) I also have this: #4905 to refer to which sounds related.

Anyhoo, thank you and good night! 🙂

@milseman
Copy link
Mannequin

milseman mannequin commented Apr 24, 2017

1) Correct.
2) The existing error is still important, you can't have an argument that is @escaping Int, for instance. Keep the current behavior in the general case, but you're checking for the special case of optional closure. "If this is an optional closure, then emit the more specific and helpful diagnostic". And I think the message should be "Optional closures are already escaping".
3) Find the diag code for the current error message, then grep the source base for uses of that diag to see where to put your special case checking logic. You can then search that section of the code for other things that might check for e.g. optionality.
4) That is a test file (hence being under the test directory). You can do a TDD-like development flow by adding the test cases where you think you should have a better diagnostic and fixit around, then make those tests pass
5) That PR does AST printing, which should be orthogonal to this effort. You'll probably be modifying files with "Sema..." or "TypeCheck..." in their names, as this diagnostic is emitted as part of type checking.

@swift-ci
Copy link
Collaborator Author

Comment by Garric Nahapetian (JIRA)

@milseman Excellent feedback. Thank you!

@swift-ci
Copy link
Collaborator Author

Comment by Kaden Wilkinson (JIRA)

Error is currently pretty cryptic in Swift 4 ![](Screen Shot 2017-09-11 at 8.03.08 AM.png)

@swift-ci
Copy link
Collaborator Author

swift-ci commented Jun 7, 2019

Comment by Michael Critz (JIRA)

#tryswift

@swift-ci
Copy link
Collaborator Author

swift-ci commented Jun 8, 2019

Comment by Garric Nahapetian (JIRA)

I thought we already fixed this: #8947

I wonder what happened...

@swift-ci
Copy link
Collaborator Author

Comment by Michael Critz (JIRA)

garricn (JIRA User) I was able to reproduce the original bug. The note didn’t appear in the fixit UI. The work is mostly done. I’d like to add some minimal logic to the fix-it to be more informative and accurate. Basically, add the suggested text from the April 2017 comments.

@swift-ci
Copy link
Collaborator Author

Comment by Garric Nahapetian (JIRA)

Great!

@belkadan
Copy link
Contributor

critz (JIRA User), are you still working on this?

(I'm checking in on all the StarterBugs that haven't been touched in over a month; it's totally fine if you just haven't had time but still want to keep it.)

@swift-ci
Copy link
Collaborator Author

Comment by Michael Critz (JIRA)

@belkadan not actively. Feel free to bump me off this.

@belkadan
Copy link
Contributor

Done. You're welcome to return to it if you get the time!

@swift-ci
Copy link
Collaborator Author

swift-ci commented Apr 6, 2020

Comment by Ivan Smetanin (JIRA)

Fixed in #30746

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants