[SR-5556] @escaping Diagnostics Should Always Offer a Fixit

[CodaFi]

Previous ID	SR-5556
Radar	None
Original Reporter	@CodaFi
Type	Bug

Additional Detail from JIRA

Votes	0
Component/s	Compiler
Labels	Bug, DiagnosticsQoI
Assignee	mkchoi212 (JIRA)
Priority	Medium

md5: 4f14ea42d651469b5c1a49e8c94f02a5

is duplicated by:

• SR-5455 Compiler not able to provide automatic fix-it for returning closure

Issue Description:

The following code

func mapping <A, B, C> (f: (A) -> (B)) -> (((C, B) -> (C))) -> ((C, A) -> (C)) {
  return { reducer in
    return { accum, input in
      reducer(accum, f(input))
    }
  }
}

Should suggest that we insert @escaping in two places such that the final signature is

func mapping <A, B, C> (f: @escaping (A) -> (B)) -> (@escaping ((C, B) -> (C))) -> ((C, A) -> (C))

However, it only chooses to offer a fixit for the leftmost @escaping and reports the rightmost one as escaping without a fixit. We should diagnose both consistently and offer a fixit.

[CodaFi]

@xedin Ping.

[CodaFi]

This one was originally reported on the list.

[swift-ci]

Comment by Mike Choi (JIRA)

I believe this is a duplicate of https://bugs.swift.org/browse/SR-5455, an issue I opened awhile ago.

[CodaFi]

Because Pavel's got this one and not the earlier one, I'll dupe the old one here to the new one (oddly enough).

[swift-ci]

Comment by Mike Choi (JIRA)

Can I give this bug a go? (if you guys haven't figured it out yet)
I want to contribute to Swift and think this could be a good starting point!

[xedin]

mkchoi212 (JIRA User) Sure thing, once you are ready feel free to assign me as a reviewer as well! Here is the idea - currently @escaping is mostly diagnosed in `MiscDiagnostics` which is ran after `FailureDiagnosis` I think we need to extract that logic into `FailureDiagnosis` itself and/or expect diagnoseContextualErrors to handle more scenarios.

[swift-ci]

Comment by Mike Choi (JIRA)

@xedin Will do and thanks for the hint. Running `update-checkout --clone` now!

[swift-ci]

Comment by Mike Choi (JIRA)

@xedin I tracked down the bug to `walkToDeclRefExpr` in `TypeCheckCaputres.cpp`. Turns out when `DRE->getDecl()` is called on a parameter of a returning closure, the parameter within the closure expression is grabbed, instead of the one from the function declaration. Because of this, when `paramDecl->getTypeLoc()` is called, no type location is provided for `fixItInsert`. And this all happens after errors regarding `@escaping` are detected within `MiscDiagnostics`.

Am I on the right track and if so, any ideas where to go from here?

[xedin]

mkchoi212 (JIRA User) I think you are on the right track but it sounds like it would be tricky to improve because fix-it which we do add is attached to the declaration of the parameter but in the second case the problem is located inside of the type (((C, B) -> (C))) -> ((C, A) -> (C)), I think what you could try to do is to figure out how to match type type in question to the type in function declaration which would have source information for you to insert @escaping.

[swift-ci]

Comment by Mike Choi (JIRA)

@xedin Hmm, this bug is starting to get more and more "non-starter". But I will give it one last go! @harlanhaskins kindly suggested I dig around the constraint system.

Did some digging and thought about doing something like this

VD->getDeclContext()>getParentForLookup()>lookupQualified(type, name, NL_QualifiedDefault | NL_KnownNoDependency, nullptr, results);

but we can't perform searches based on names here. Are there any other ways to do lookups?

[xedin]

mkchoi212 (JIRA User) I think we should let this be for a while because we need a better way in diagnostics to detect situations like this. I would suggest you to tackle https://bugs.swift.org/browse/SR-910 instead, which seems to be more straight-forward which what we have right now.

[swift-ci]

Comment by Mike Choi (JIRA)

@xedin Ok, that sounds great. And thanks for the link to the new bug. I will get started on that ASAP

[xedin]

mkchoi212 (JIRA User) No problem, take your time! If you need any help just let me know if that issue.

[swift-ci]

Comment by Mike Choi (JIRA)

@xedin Noted and will do! 👍