Skip to content

Improve the error message for passing (any P)? to (some P)? #61733

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

Open
hborla opened this issue Oct 26, 2022 · 23 comments
Open

Improve the error message for passing (any P)? to (some P)? #61733

hborla opened this issue Oct 26, 2022 · 23 comments
Assignees
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler itself diagnostics QoI Bug: Diagnostics Quality of Implementation existentials Feature: values of types like `any Collection`, `Any` and `AnyObject`; type-erased values good first issue Good for newcomers type checker Area → compiler: Semantic analysis

Comments

@hborla
Copy link
Member

hborla commented Oct 26, 2022

If you try to pass a value of type (any P)? to an argument of type (some P)?, the compiler tells you that any P cannot conform to P, which is confusing and not actionable. With SE-0375, existential opening/unboxing is supported with optionals, but only if the argument is not an optional because it's possible that the (any P)? is nil (and thus does not have a dynamic type to bind to some P). With SE-0375, the fix is to unwrap the optional or provide a default value. So, the error message should tell you that!

protocol P {}

struct S: P {}

func takesOptionalP(_: (some P)?) {}

func passOptional(value: (any P)?) {
  takesOptionalP(value) // error: type 'any P' cannot conform to 'P'

  takesOptionalP(value ?? S()) // okay under SE-0375
  takesOptionalP(value!) // okay under SE-0375
}

The constraint system currently applies the MissingConformance fix when solving for the takesOptionalP(value) expression. Instead, it should identify that the wrapped type of the argument matches the parameter type, and apply the ForceOptional fix.

@hborla hborla added the bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. label Oct 26, 2022
@hborla
Copy link
Member Author

hborla commented Oct 26, 2022

@dfperry5 this is very similar to #60730 in case you're interested in taking a look at this one too 🙂

@hborla hborla added the good first issue Good for newcomers label Oct 26, 2022
@LucianoPAlmeida LucianoPAlmeida added diagnostics QoI Bug: Diagnostics Quality of Implementation type checker Area → compiler: Semantic analysis labels Oct 26, 2022
@IceCurrent
Copy link

IceCurrent commented Oct 27, 2022

@hborla, I would love to work on it. But I'm completely new to this repo (and open source contribution in general). It would be very kind of you tell me how do I go about it

@hborla
Copy link
Member Author

hborla commented Oct 27, 2022

@IceCurrent welcome to the Swift project! I'm happy you're interested in working on this issue 🙂

Are you set up to build the Swift compiler yet? There are a few getting started guides:

The first one contains instructions for checkout and build the Swift compiler, and the second has an FAQ for new contributors. Let me know once you've gotten set up and I can provide some guidance on investigating this issue!

@enathang
Copy link

enathang commented Nov 8, 2022

@IceCurrent Hey! Are you still working on this?

I can take this fix up if you're no longer interested.

@IceCurrent
Copy link

@hborla I'm really sorry I could not work on it right now. There has been some family emergency, which I'm busy dealing with. @enathang It would be great if you could take up this issue.
Sorry once again!

@hborla
Copy link
Member Author

hborla commented Nov 10, 2022

No worries at all! @enathang please let me know if you have questions or need guidance!

@enathang
Copy link

Thanks @hborla! Currently waiting on a new external hard drive because my computer ran out of space trying to build the project. In the mean time, I looked over the documentation (specifically the overview of constraint system) and this commit. If you have any additional info on the constraint system feel free to pass along, it seems cool!

Will update once I have the project built and start poking through files.

@enathang
Copy link

@hborla Is there any info on reducing the size requirement of Swift/xcode/swift toolchain? I'm wondering if I'd like to only work on a part of the toolchain (ie type constraint) I could swap out other parts of the toolchain with compiled versions to reduce space.

I have a 2017 128GB macbook air. I've stripped out every non-swift related file on my computer to free space and I'm still running into disk space issues. Your thoughts would be much appreciated!

@enathang
Copy link

enathang commented Nov 22, 2022

Managed to get everything working through a flash drive.

I've done a bit of testing to determine the behavior of the compiler currently:
Passing any P to some P -> No error
Passing any P to (some P)? -> No error

Passing (any P)? to (some P)? -> NonConformance (should be UnwrapLiteral per solving this issue)
Passing (any P)? to some P -> Both NonConformance and UnwrapLiteral

I'll keep looking into this.

@AnthonyLatsis AnthonyLatsis added the compiler The Swift compiler itself label Dec 13, 2022
@NuriAmari
Copy link
Contributor

@enathang Are you still working on this? If not, I'd like to give it a try.

@enathang
Copy link

enathang commented Feb 1, 2023

Go for it! I worked on it for a while but never quite figured it out.

@AnthonyLatsis AnthonyLatsis added the existentials Feature: values of types like `any Collection`, `Any` and `AnyObject`; type-erased values label Feb 1, 2023
@NuriAmari NuriAmari self-assigned this Feb 1, 2023
@NuriAmari
Copy link
Contributor

Hi @LucianoPAlmeida @hborla @AnthonyLatsis, I've given this a try but could use some pointers if someone has the time.

The crux of the issue I'm running into is convincing the type checker to choose the ForceOptional solution over the MissingConformance solution. The change introduced in #61191 already makes it so that the ForceOptional solution is considered, but the solver bails before fully considering the solution because it is strictly worse than the chosen MissingConformance solution, due to the value-to-optional promotion penalty.

We can see this in the constraint debugging output:

---Attempting to salvage and emit diagnostics---
  (considering -> $T1 conforms to P [[locator@0x11d9ac6d8 [DeclRef@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> opened generic -> type parameter requirement #0 (conformance)]]];
    (simplification result:
    )
    (outcome: unsolved)
  )
  (attempting disjunction choice (P)? bind ($T1)? [deep equality] [[locator@0x11d9ac9f0 [Call@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> apply argument -> comparing call argument #0 to parameter #0]]];
    (considering -> $T1 conforms to P [[locator@0x11d9ac6d8 [DeclRef@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> opened generic -> type parameter requirement #0 (conformance)]]];
      (simplification result:
    (attempting fix [fix: add missing protocol conformance] @ locator@0x11d9ac6d8 [DeclRef@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> opened generic -> type parameter requirement #0 (conformance)])
    (increasing 'applied fix' score by 1)
        (removed constraint: $T1 conforms to P [[locator@0x11d9ac6d8 [DeclRef@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> opened generic -> type parameter requirement #0 (conformance)]]];)
      )
      (outcome: simplified)
    )
    (Changes:
      (Newly Bound:
        > $T1 := (P)
      )
      (Removed Constraint:
        > $T1 conforms to P [[locator@0x11d9ac6d8 [DeclRef@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> opened generic -> type parameter requirement #0 (conformance)]]];
      )
    )
    (found solution: [component: applied fix(s), value: 1])
  )
  (attempting disjunction choice (P)? arg conv ($T1)? [optional-to-optional] [[locator@0x11d9ac9f0 [Call@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> apply argument -> comparing call argument #0 to parameter #0]]];
        (added constraint: (P) arg conv $T1 [[locator@0x11d9acc40 [Call@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> apply argument -> comparing call argument #0 to parameter #0 -> generic argument #0]]];)
    (Changes:
      (Added Constraint:
        > (P) arg conv $T1 [[locator@0x11d9acc40 [Call@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> apply argument -> comparing call argument #0 to parameter #0 -> generic argument #0]]];
      )
    )
    (Potential Binding(s):
      ($T1 [attributes: potentially_incomplete] [with possible bindings: (supertypes of) P])
    )
    (attempting type variable $T1 := P
      (considering -> $T1 conforms to P [[locator@0x11d9ac6d8 [DeclRef@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> opened generic -> type parameter requirement #0 (conformance)]]];
        (simplification result:
      (attempting fix [fix: add missing protocol conformance] @ locator@0x11d9ac6d8 [DeclRef@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> opened generic -> type parameter requirement #0 (conformance)])
      (increasing 'applied fix' score by 1)
          (removed constraint: $T1 conforms to P [[locator@0x11d9ac6d8 [DeclRef@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> opened generic -> type parameter requirement #0 (conformance)]]];)
        )
        (outcome: simplified)
      )
      (considering -> (P) arg conv $T1 [[locator@0x11d9acc40 [Call@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> apply argument -> comparing call argument #0 to parameter #0 -> generic argument #0]]];
        (simplification result:
          (removed constraint: (P) arg conv $T1 [[locator@0x11d9acc40 [Call@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> apply argument -> comparing call argument #0 to parameter #0 -> generic argument #0]]];)
        )
        (outcome: simplified)
      )
      (Changes:
        (Newly Bound:
          > $T1 := P
        )
        (Removed Constraints:
          > $T1 conforms to P [[locator@0x11d9ac6d8 [DeclRef@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> opened generic -> type parameter requirement #0 (conformance)]]];
          > (P) arg conv $T1 [[locator@0x11d9acc40 [Call@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> apply argument -> comparing call argument #0 to parameter #0 -> generic argument #0]]];
        )
      )
      (found solution: [component: applied fix(s), value: 1])
    )
  )
  (attempting disjunction choice (P)? arg conv ($T1)? [value-to-optional] [[locator@0x11d9ac9f0 [Call@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> apply argument -> comparing call argument #0 to parameter #0]]];
    (increasing 'value to optional promotion' score by 1)
        (added constraint: (P)? arg conv $T1 [[locator@0x11d9accf0 [Call@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> apply argument -> comparing call argument #0 to parameter #0 -> optional payload]]];)
    (Changes:
      (Added Constraint:
        > (P)? arg conv $T1 [[locator@0x11d9accf0 [Call@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> apply argument -> comparing call argument #0 to parameter #0 -> optional payload]]];
      )
    )
    (Potential Binding(s):
      ($T1 [attributes: potentially_incomplete] [with possible bindings: (supertypes of) (P)?])
    )
    (attempting type variable $T1 := (P)?
      (considering -> $T1 conforms to P [[locator@0x11d9ac6d8 [DeclRef@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> opened generic -> type parameter requirement #0 (conformance)]]];
        (simplification result:
      (attempting fix [fix: add missing protocol conformance] @ locator@0x11d9ac6d8 [DeclRef@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> opened generic -> type parameter requirement #0 (conformance)])
      (increasing 'applied fix' score by 1)
      (solution is worse than the best solution)
      (attempting fix [fix: add missing protocol conformance] @ locator@0x11d9ac6d8 [DeclRef@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> opened generic -> type parameter requirement #0 (conformance)])
      (increasing 'applied fix' score by 3)
      (solution is worse than the best solution)
          (removed constraint: $T1 conforms to P [[locator@0x11d9ac6d8 [DeclRef@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> opened generic -> type parameter requirement #0 (conformance)]]];)
          (failed constraint $T1 conforms to P [[locator@0x11d9ac6d8 [DeclRef@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> opened generic -> type parameter requirement #0 (conformance)]]];)
        )
        (outcome: error)
      )
    )
    (attempting type variable $T1 := (P)
      (considering -> (P)? arg conv $T1 [[locator@0x11d9accf0 [Call@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> apply argument -> comparing call argument #0 to parameter #0 -> optional payload]]];
        (simplification result:
      (attempting fix [fix: force optional] @ locator@0x11d9accf0 [Call@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> apply argument -> comparing call argument #0 to parameter #0 -> optional payload])
      (increasing 'applied fix' score by 1)
      (solution is worse than the best solution)                                                                      !!!!!!!!!!             :(          !!!!!!!!!!!!!!!!!!!
          (removed constraint: (P)? arg conv $T1 [[locator@0x11d9accf0 [Call@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> apply argument -> comparing call argument #0 to parameter #0 -> optional payload]]];)
          (failed constraint (P)? arg conv $T1 [[locator@0x11d9accf0 [Call@/Users/nuriamari/git/swift-project/swift/test/Constraints/scratch.swift:10:3 -> apply argument -> comparing call argument #0 to parameter #0 -> optional payload]]];)
        )
        (outcome: error)
      )
    )
  )

Strategies I've tried so far:

  • Reduce the impact of the ForceOptional fix from 1 to 0
    • At least convinced the solver not to bail early, but it still required other fixes and seems like a hack with unwanted side effects
  • Preventing the consideration of MissingConformance in the first place.
    • I found that in the locations where binding type variable T1 to concrete type P was being considered, I didn't have access to enough context to know that an (any P)? -> (some P)? conversion was being attempted.

Some general questions I have, again if there is time:

  • In the debug output, I see "active constraints" and "inactive" constraints
    • What makes a constraint active vs inactive?
    • Perhaps related, how does the typechecker pick a starting constraint to begin simplifying. From the documentation I understand it computes connected components in the constraint graph, and independently solves each component, but its not clear to me how it picks which constraint within a component to start with.
  • How closely does the typechecker resemble Hindley-Milner? Would it be worth it / recommended to read some of the literature around it.

@LucianoPAlmeida
Copy link
Contributor

Preventing the consideration of MissingConformance in the first place.
I found that in the locations where binding type variable T1 to concrete type P was being considered, I didn't have access to enough context to know that an (any P)? -> (some P)? conversion was being attempted.

@hborla or @xedin may be the best advice here, but from what I could get from looking at it is that we are recording a missing conformance from an existential any P and protocol P (with an additional optional-to-optional or deep-equality restriction) which doesn't look correct.
So maybe we should detect that existential case in fixRequirementFailure taking into account the restrictions as well and produce different fix.

Also @hborla, it looks like the second case still fails https://godbolt.org/z/bzeP1EKM5. Is that a bug?

@Rajveer100
Copy link
Contributor

This issue seems really interesting to grind! Let me try and find a solution to it!

@LucianoPAlmeida
Copy link
Contributor

This issue seems really interesting to grind! Let me try and find a solution to it!

You definitely can, but I think this issue has an assignee already. So we always recommend to ask the current assignee (or person who commented that is working on it) first. See How do I pick something to work on for more details.

@NuriAmari
Copy link
Contributor

This issue seems really interesting to grind! Let me try and find a solution to it!

I'll still be working on it, but I have other things on my plate as well, so if you find a solution first, that's fine too 🙂

@Rajveer100
Copy link
Contributor

Cool!

@NuriAmari
Copy link
Contributor

Small update for those interested:

Perhaps this was clear to others, but it wasn't to me; the reason we are seeing a MissingConformance fix is because we are not opening the existential Any P and P does not in general conform to itself.

This means unless we open the existential, we will always have to apply the MissingConformance fix.

I'm trying now to conditionally open the existential only when applying fixes, and after applying the ForceOptional fix, but I haven't quite figured out how to do this.

NuriAmari added a commit to NuriAmari/swift that referenced this issue Feb 15, 2023
NuriAmari added a commit to NuriAmari/swift that referenced this issue Feb 15, 2023
@NuriAmari
Copy link
Contributor

I've created a draft PR here: #63667. It by no means solves the issue, but I've written a comment explaining where I'm struggling.

I will take another stab at it sometime later when I have the time, but any pointers would be appreciated @LucianoPAlmeida @hborla @AnthonyLatsis 🙂

Also @Rajveer100 feel free to chime in if you have any ideas.

@Rajveer100
Copy link
Contributor

Well, I am grinding as of now!

@Rajveer100
Copy link
Contributor

Rajveer100 commented Feb 15, 2023

I think changes in these files could affect the error right?

lib/Sema/CSDiagnostics.cpp:  emitDiagnostic(diag::type_cannot_conform,
lib/Sema/CSDiagnostics.cpp:        emitDiagnostic(diag::type_cannot_conform, fromType, toType);
lib/Sema/TypeCheckProtocol.cpp:      diags.diagnose(ComplainLoc, diag::type_cannot_conform,
lib/Sema/TypeCheckProtocol.cpp:          diags.diagnose(ComplainLoc, diag::type_cannot_conform_to_nsobject,

Particularly, this function:
void swift::diagnoseConformanceFailure

@NuriAmari
Copy link
Contributor

NuriAmari commented Feb 15, 2023

I think changes in these files could affect the error right?

In my opinion, you should be looking farther from the the diagnostic emission call sites, around the CSSimplify.cpp and CSSolver.cpp area off the top of my head. We need to modify which fixes the constraint system applies, not how it emits diagnostics for them after having decided which to apply. More concretely, we need to stop applying a conformance failure fix, not modify how a conformance failure is diagnosed 🙂

Random collection of resources I used to get some understanding of existential types:

https://developer.apple.com/videos/play/wwdc2016/416/
https://developer.apple.com/videos/play/wwdc2022/110352
https://github.com/apple/swift-evolution/blob/main/proposals/0352-implicit-open-existentials.md#when-can-we-open-an-existential
https://github.com/apple/swift-evolution/blob/main/proposals/0375-opening-existential-optional.md
https://forums.swift.org/t/very-confused-about-some-compiler-warnings-related-to-protocols-bugs-or-intended/39754
https://forums.swift.org/t/allowing-self-conformance-for-protocols/39841

@Rajveer100
Copy link
Contributor

Thanks! @NuriAmari

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler itself diagnostics QoI Bug: Diagnostics Quality of Implementation existentials Feature: values of types like `any Collection`, `Any` and `AnyObject`; type-erased values good first issue Good for newcomers type checker Area → compiler: Semantic analysis
Projects
None yet
Development

No branches or pull requests

7 participants