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

[CodaFi]

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]

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]

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]

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]

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]

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

[belkadan]

Resetting assignee on all Starter Bugs not modified since 2018.