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-1807] Sema should catch and warn about self capture in closures #44416

Open
CodaFi opened this issue Jun 17, 2016 · 6 comments
Open

[SR-1807] Sema should catch and warn about self capture in closures #44416

CodaFi opened this issue Jun 17, 2016 · 6 comments
Labels
compiler The Swift compiler in itself good first issue Good for newcomers new feature

Comments

@CodaFi
Copy link
Member

CodaFi commented Jun 17, 2016

Previous ID SR-1807
Radar rdar://26865978
Original Reporter @CodaFi
Type New Feature
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels New Feature, StarterBug
Assignee None
Priority Medium

md5: 3ea294e377274fa3d27e3c3b76ff7bbd

Issue Description:

To maintain parity with Objective-C, we should use our ARC analysis pass in Sema to check for the presence of self capture in blocks without weak or @noescape. This should be a very simple warning modeled after the one in /clang/lib/Sema/SemaChecking.cpp void diagnoseRetainCycle().

@swift-ci
Copy link
Collaborator

Comment by Paul Meng (JIRA)

I am interested into working on this one. It would be great to give some guidances on status quo on self retain cycle detection.

I started by searching the keyword `ARC and found `ARCAnalysis.cpp`, but it looks the functions inside are not referenced from outside and mostly work on `SILInstruction`, and it looks like currently there is nothing comparable to the `findRetainCycleOwner` or `checkRetainCycles` from `/clang/lib/Sema/SemaChecking.cpp`. Therefore I don't think `ARCAnalysis.cpp` is the way to go. then I tried to `ag` in the directory to find something sound like `retain`, `cycle` etc but I am unlucky and couldn't find anything.

so the questions are

  1. Where is the ARC analysis pass code are located? or should we implement it from scratch that is one local walk of the tree on closure expression?
  2. Related to 1. Most of the code in the Sema are traversing the AST and collecting the constraints and solve it locally or globally. What would be the best way to put the ARC code without being intrusive?

Thanks!

@CodaFi
Copy link
Member Author

CodaFi commented Dec 31, 2016

PaulMeng (JIRA User) So sorry it took me so long to get to this. I've started a branch with something that handles quite a few cases already if you'd like to pick it up and carry it over the goal line.

@swift-ci
Copy link
Collaborator

Comment by Paul Meng (JIRA)

CodaFi (JIRA User) Neat! Thanks for laying the ground work for newbie here. I briefly checked your branch and I think it is quite complete. probably what are left are test cases before carrying it over the goal line?

I don't think there is a spec for this, I could compare it against clang's Sema code http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?revision=290146&view=markup ti check if there is any critical cases left.

@swift-ci
Copy link
Collaborator

swift-ci commented Jan 3, 2017

Comment by Paul Meng (JIRA)

It seems that it would report false alarm on this snippet. I am trying to propose a fix for that.
```
class A {
let name: String

init(name: String) {
let foo = { return self.name }
_ = foo()
}
}
```

```
diag_retain_cycle.swift:18:29: warning: capturing 'self' strongly in this block is likely to lead to a retain cycle
let foo = { return self.name }
^
diag_retain_cycle.swift:18:15: warning: block will be retained by an object strongly retained by the captured object
let foo = { return self.name }
```

@hartbit
Copy link
Collaborator

hartbit commented Apr 26, 2018

PaulMeng (JIRA User) are you still working on this? I wouldn't mind picking it up.

@belkadan
Copy link
Contributor

Resetting assignee on all Starter Bugs not modified since 2018.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler The Swift compiler in itself good first issue Good for newcomers new feature
Projects
None yet
Development

No branches or pull requests

4 participants