I don't think "non-optional variant" is correct terminology. How about something like:

warning: left side of nil coalescing operator '??' is implicitly converted from type 'T?' to 'T??', so the right side is never used

You can change the diagnostic in DiagnosticsSema.def (tip: diagnostic names show up if you pass -Xfrontend -debug-diagnostic-names to the compiler) and fix any related test cases.

Comment by Jack March (JIRA)

thanks theindigamer (JIRA User) for the tips and the suggestion! One thing I would note though is how changing the diagnostic to that would behave for this function:

func foo<T>(
    t: T,
    t2: T
) -> T? {
    t ?? t2
}

Current diagnostic:

warning: Left side of nil coalescing operator '??' has non-optional type 'T', so the right side is never used

New diagnostic:

warning: left side of nil coalescing operator '??' is implicitly converted from type 'T' to 'T?', so the right side is never used

I think the former is easier to understand because it explicitly points out we're using the nil-coalescing operator for a non-optional type.

As an alternative, does this have correct terminology?

func foo<T>(
    optionalT: T?,
    secondOptionalT: T?
) -> T?? {
    optionalT ?? secondOptionalT // Warning: Left side of nil coalescing operator '??' has type 'T?' (non-optional T??), so the right side is never used
}

func foo<T>(
    t: T,
    t2: T
) -> T? {
    t ?? t2 // Warning: Left side of nil coalescing operator '??' has type 'T' (non-optional 'T?'), so the right side is never used
}

I wonder if this is the expected behavior to implicit wrap it in an implicit inject_into_optional in this case? This is just a question of curiosity, because to me for example:

func practicalExample<T>(dict: inout [String:T?],
                         optionalT: T?,
                         secondOptionalT: T?
) {
  // 1.
  dict["foo"] = optionalT ?? secondOptionalT // Warning: Left side of nil coalescing operator '??' has non-optional type 'T?', so the right side is never used
  let _: T?? = optionalT ?? secondOptionalT // Warning: Left side of nil coalescing operator '??' has non-optional type 'T?', so the right side is never used
  
  // 2.
  let a = optionalT ?? secondOptionalT 
  dict["bar"] = a
  let _: T?? = a 
}

1 and 2 are in behavior(intention of user) expected to be the same, but because of the implicit injection on optional to optional the behavior is ends up being a bit unexpected and surprising from user perspective... I'm not familiar with the details and why's of it but I wonder if the implicit injection is necessary in this case?

cc @xedin CodaFi (JIRA User)

Unfortunately it's just a consequence of argument application, since ?? has signature <T>(_: T?, _: @autoclosure () throws -> T) -> T solver ends up matching T? (as an argument) to $T_param? which creates this implicit injection for the first argument.

Hum, yeah ... this makes sense, but given both first and second arguments are `T?` I wonder if would make sense to pick the optional overload

That would make sense to me, I wonder what would it take to change optional promotion behavior when it comes to type variables, if that's easy to do we should try to run source compat suite and see if it breaks anything. With that adjustment constraints like `$T? conv Int?` would result in `$T` bound to `Int` without any additional promotions.

@xedin @LucianoPAlmeida how about we improve the diagnostic here, and split the work of investigating the type-checking change in a separate JIRA? It seems to me that the diagnostic change has negligible risk and is useful, so it makes sense to land that first. The type-checking change is potentially riskier (even if the source compat suite passes, some other code could fail), so splitting out that into a separate JIRA unblocks work on this.

I think this would make sense theindigamer (JIRA User), and also just improve the diagnostic wording in those special cases seems like a good starter task.
What do you think @xedin?

Sounds good to me if that would be easy to detect.

As far as I understand by a quick look to just improve the wording on diagnostic the change would be here

Yeah, that does make sense to me, it definitely makes it easier to check on the type-checked AST.

Right, so let's make it a starter bug, thanks for the input @xedin

Hi @xedin @LucianoPAlmeida @typesanitizer, as my first issue, I would like to handle this task. I understood that the request is to change the diagnostic for T? ?? T? without affecting the other cases, is that correct?

I was also wondering what should be the right diagnostic in this case

func foo<T>(
    t: T,
    t2: T?
) -> T?? {
    t ?? t2
}

Sorry I think this one shouldn't have "good first issue" label, to fix it you'd need a pretty deep understanding of the solver.

Thank you @xedin, I've tried it and found a solution that implements the diagnostic suggested by @typesanitizer. However, if you say this is hard, my solution probably isn't right or breaks something else, I'll try another issue!

That's what usually happens with situations like this. I think the fix here would be to prevent type matcher from creating any restrictions except to DeepEquality if both types are known to be Optional.