swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.2k stars 10.32k forks source link

Closure Evaluation Misdiagnoses Initializer Type Checking #62051

Open Mordil opened 1 year ago

Mordil commented 1 year ago

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)

Additional context

N/A

AnthonyLatsis commented 1 year ago

Reduction for the first example:

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

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

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

  return Foo(0)
}
simanerush commented 1 year ago

I'll take a look!

simanerush commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

@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.