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

Rule Request: unowned_captures #2097

Closed
2 tasks done
mdiep opened this issue Mar 14, 2018 · 1 comment · Fixed by #2739
Closed
2 tasks done

Rule Request: unowned_captures #2097

mdiep opened this issue Mar 14, 2018 · 1 comment · Fixed by #2739
Labels
rule-request Requests for a new rules.

Comments

@mdiep
Copy link

mdiep commented Mar 14, 2018

New Issue Checklist

Rule Request

  1. Why should this rule be added? Share links to existing discussion about what
    the community thinks about this.

Dunno if there's general consensus on this, but I have a strong preference to use [weak foo] captures over [unowned foo] captures. While weak can have performance implications and can require some additional code, it's safer than unowned. I've encountered quite a few mysterious crashes that were caused by unowned captures, even in cases where it seemed like the lifetimes were the same.

  1. Provide several examples of what would and wouldn't trigger violations.

Triggering:

{ [unowned self] in _ }()

Non-triggering:

{ [weak self] in _ }()
  1. Should the rule be configurable, if so what parameters should be configurable?

Nope.

  1. Should the rule be opt-in or enabled by default? Why?
    See README.md for guidelines on when to mark a
    rule as opt-in.

I'm not sure if there's enough consensus about this to enable it by default.

@marcelofabri marcelofabri added the rule-request Requests for a new rules. label Mar 17, 2018
@marcelofabri
Copy link
Collaborator

Swift 4.1 provides that information in SourceKit structure:

{
  "key.diagnostic_stage" : "source.diagnostic.stage.swift.parse",
  "key.length" : 31,
  "key.offset" : 0,
  "key.substructure" : [
    {
      "key.bodylength" : 0,
      "key.bodyoffset" : 30,
      "key.kind" : "source.lang.swift.expr.call",
      "key.length" : 31,
      "key.name" : "foo() { [unowned self] in _ }",
      "key.namelength" : 29,
      "key.nameoffset" : 0,
      "key.offset" : 0,
      "key.substructure" : [
        {
          "key.bodylength" : 24,
          "key.bodyoffset" : 4,
          "key.kind" : "source.lang.swift.expr.call",
          "key.length" : 29,
          "key.name" : "foo",
          "key.namelength" : 3,
          "key.nameoffset" : 0,
          "key.offset" : 0,
          "key.substructure" : [
            {
              "key.attributes" : [
                {
                  "key.attribute" : "source.decl.attribute.weak",
                  "key.length" : 0,
                  "key.offset" : 0
                }
              ],
              "key.kind" : "source.lang.swift.decl.var.local",
              "key.length" : 4,
              "key.name" : "self",
              "key.namelength" : 4,
              "key.nameoffset" : 17,
              "key.offset" : 17
            },
            {
              "key.attributes" : [
                {
                  "key.attribute" : "source.decl.attribute.weak",
                  "key.length" : 0,
                  "key.offset" : 0
                }
              ],
              "key.kind" : "source.lang.swift.decl.var.local",
              "key.length" : 4,
              "key.name" : "self",
              "key.namelength" : 4,
              "key.nameoffset" : 17,
              "key.offset" : 17
            }
          ]
        }
      ]
    }
  ]
}

However, this will probably not be enough because the attribute range for unowned is wrong 😭

I've filled https://bugs.swift.org/browse/SR-7219 to track this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule-request Requests for a new rules.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants