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

Closure Evaluation Misdiagnoses Initializer Type Checking #62051

Open
Mordil opened this issue Nov 11, 2022 · 5 comments
Open

Closure Evaluation Misdiagnoses Initializer Type Checking #62051

Mordil opened this issue Nov 11, 2022 · 5 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. closures Feature: closures compiler The Swift compiler in itself diagnostics QoI Bug: Diagnostics Quality of Implementation multi-statement closures Feature → closures: multi-statement closures swift 5.9 type checker Area → compiler: Semantic analysis type inference Feature: type inference

Comments

@Mordil
Copy link

Mordil commented Nov 11, 2022

Describe the bug

When using a generic initializer of a type that is returned in a compactMap closure, the compiler gives misleading errors as part of the closure evaluation.

Steps To Reproduce

I found two separate reproduction cases that give two different errors

First Reproduction

struct FinalContainer {
  init(boolSequence: [Bool], intArray: [Int]) { }
}

let intArray = repeatElement(1, count: 2)

var containers: [FinalContainer] = []
containers = (0...3).compactMap { _ in
  // the guard itself doesn't matter, it's just to have the control statement to return 'nil'
  guard false else {
    return nil
        // ^^^^^^ 'nil' is incompatible with return type 'FinalContainer'
  }

  return FinalContainer(boolSequence: intArray, intArray: intArray)
}

Note: Changing intArray to be intArray = [1] causes the compiler to correctly diagnose the type mismatch on the FinalContainer initializer

Second Reproduction

struct FinalRepresentation {
  init(boolSequence: some Sequence<Bool>, intArray: [Int]) { }
}

let secondType = [1]

var storage: [FinalRepresentation] = []
storage = (0...3).compactMap { _ in
     // ^^^^^^ Type of expression is ambiguous without more context
  return FinalRepresentation(boolSequence: secondType, intArray: secondType)
}

Note: Changing boolSequence to be [Bool] causes the compiler to correctly diagnose the type mismatch on the FinalContainer initializer

Expected behavior

The compiler to correctly diagnose the type mismatch on the initializer: Cannot convert value of type '[Int]' to expected argument type '[Bool]'

Screenshots

N/A

Environment (please fill out the following information)

  • OS: macOS 12.6
  • Xcode Version/Tag/Branch: Xcode 14.0.1 (14A400)

Additional context

N/A

@Mordil Mordil added the bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. label Nov 11, 2022
@xedin xedin added type checker Area → compiler: Semantic analysis diagnostics QoI Bug: Diagnostics Quality of Implementation labels Nov 11, 2022
@AnthonyLatsis
Copy link
Collaborator

AnthonyLatsis commented Nov 12, 2022

Reduction for the first example:

struct Foo {
  init(_: Bool) {}
}

func foo(_: () -> Foo?) {}

//let _: () -> Foo? = { // Correct diagnostic
foo {
  if true {
    return nil
  }

  return Foo(0)
}

@AnthonyLatsis AnthonyLatsis added compiler The Swift compiler in itself closures Feature: closures labels Nov 15, 2022
@simanerush
Copy link
Member

I'll take a look!

@simanerush
Copy link
Member

@xedin I've looked at this issue, and there seems to be an issue with the constraint solver. If we take the code @AnthonyLatsis provided and further simplify it like so,

func foo(_: () -> Bool?) {}

// let _: () -> Bool? = { // Correct diagnostic
foo {
  if true {
    return nil
  }
  return 0
}

we can then print out all the solutions that the constraint solver found. In both scenarios, the correct solution is found with value 3. But in the incorrect case, the solver finds another winning solution with value 1 that looks like this: Fixed score: [component: applied fix(s), value: 1] [component: function conversion(s), value: 1] [component: value to optional promotion(s), value: 1]. This infers the type of the function to be Bool and emits wrong diagnostics about nil not being compatible with type Bool. I'm not sure where to look for implementation of the actual solving process that comes to this conclusion, and what I should look at to investigate this issue further.

@xedin
Copy link
Member

xedin commented Jan 9, 2023

You can anchor your search on this lines in the debug output:

      (attempting conjunction element syntactic element
        (return_stmt
          (integer_literal_expr type='<null>' value=0 builtin_initializer=**NULL** initializer=**NULL**))

This is where solving for return 0 happens.

First solver binding contextual (result) type to Bool? (this is the line (attempting type variable $T2 := Bool? where $T2 is the result type of the closure) and that results in a contextual failure with "impact" of 3, and afterwards it tries to re-solve return 0 with contextual type of (attempting $T2 := Bool) Bool (because it's allowed to unwrap the optional in certain circumstances).

To fix this issue we need to to make sure that the original fix where contextual type is Bool? has lower impact than the one where result type gets bound to unwrapped Bool.

@AnthonyLatsis
Copy link
Collaborator

AnthonyLatsis commented Feb 6, 2023

@xedin I am trying to wrap my head around this so I can help Sima. Do you mind talking this out with me?


So there are 2 solutions we end up with in diagnostics (salvage) mode. Here is an overview of the debug output with a focus on fixes:

Solution 1 (closure result is Bool?) Solution 2 (closure result is Bool)
return nil contextual mismatch with impact of 1 via
simplifying ExpressibleByNilLiteral conformance
constraint
return 0 contextual mismatch with impact of 3 via
simplifying conversion constraint
Skipped?!

I assume we can get away with increasing the impact of the contextual mismatch with the ExpressibleByNilLiteral conformance failure in mind, as you suggested (though it kind of sounds fragile), but I am wondering why return 0 is not solved at all for Bool. Were it solved in addition to return nil, the resulting cumulative score would put the second solution out of question, without having to do speculative impact tweaks.

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. closures Feature: closures compiler The Swift compiler in itself diagnostics QoI Bug: Diagnostics Quality of Implementation multi-statement closures Feature → closures: multi-statement closures swift 5.9 type checker Area → compiler: Semantic analysis type inference Feature: type inference
Projects
None yet
Development

No branches or pull requests

4 participants