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

Jump to bottom

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

Open
CodaFi opened this issue Jul 26, 2017 · 14 comments
Open

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

CodaFi opened this issue Jul 26, 2017 · 14 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself diagnostics QoI Bug: Diagnostics Quality of Implementation

Comments

@CodaFi
Copy link
Member

CodaFi commented Jul 26, 2017

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
Copy link
Member Author

CodaFi commented Jul 26, 2017

@xedin Ping.

@CodaFi
Copy link
Member Author

CodaFi commented Jul 26, 2017

This one was originally reported on the list.

@swift-ci
Copy link
Collaborator

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
Copy link
Member Author

CodaFi commented Jul 26, 2017

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
Copy link
Collaborator

swift-ci commented Aug 9, 2017

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
Copy link
Member

xedin commented Aug 9, 2017

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
Copy link
Collaborator

swift-ci commented Aug 9, 2017

Comment by Mike Choi (JIRA)

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

@swift-ci
Copy link
Collaborator

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
Copy link
Member

xedin commented Aug 14, 2017

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
Copy link
Collaborator

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
Copy link
Member

xedin commented Aug 16, 2017

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
Copy link
Collaborator

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
Copy link
Member

xedin commented Aug 17, 2017

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

@swift-ci
Copy link
Collaborator

Comment by Mike Choi (JIRA)

@xedin Noted and will do! 👍

@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
Assignees
No one assigned
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself diagnostics QoI Bug: Diagnostics Quality of Implementation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@xedin @CodaFi @swift-ci