[SR-8170] IUOs should trap with a distinct error message

[beccadax]

Previous ID	SR-8170
Radar	None
Original Reporter	@beccadax
Type	Improvement
Status	Closed
Resolution	Done

Additional Detail from JIRA

Votes	0
Component/s	Compiler, Standard Library
Labels	Improvement, AffectsABI
Assignee	@beccadax
Priority	Medium

md5: 2dd2dc20cece34d956473f7aaa2869ac

Issue Description:

When we trap due to a failed force-unwrap, there are two possible causes: either the user wrote a force-unwrap explicitly, or the compiler inserted a force-unwrap for an implicitly unwrapped optional. Explicit force-unwraps are common and easy to notice; IUOs are more rare but much harder to debug (since the places you might unwrap are invisible).

We should trap with a different error messages for explicit and implicit unwraps. This would help users decide whether they should examine the visible `!` characters in their code or audit it for IUOs they didn't account for.

This may affect ABI if we implement it by adding a parameter to the _diagnoseUnexpectedNilOptional function.

[beccadax]

Quick notes in five minutes of looking at the compiler:

• ForceValueExpr already includes an isForceOfImplicitlyUnwrappedOptional() getter, but some sites in CSApply.cpp don't seem to set it when they should ❓.

• Swift._diagnoseUnexpectedNilOptional calls are emitted by SILGenFunction::emitPreconditionOptionalHasValue().

• The AST node is in the SILLocation, but looking there feels like a layering violation to me, so I assume that's a bad way to implement it.

• There are two paths to that function, one for lvalues and one for rvalues, and the "is this an IUO?" bit would have to be preserved along both of them.

[belkadan]

cc @jckarter

[jckarter]

Adding an argument to _diagnoseUnexpectedNilOptional and having emitPreconditionOptionalHasValue pass the appropriate value when it forms the call sounds reasonable to me.

[beccadax]

@jckarter: Is figuring out whether a given use is an implicit force unwrap by looking at the node in the SILLocation a decent implementation or an ugly hack? I could add it as a parameter to SILGenFunction::emitCheckedGetOptionalValueFrom(), but some of its callers look like very general conversion code that would have a hard time knowing what to pass. (Although maybe a force-unwrap would never pass through those parts of SILGen...hmm...)

[jckarter]

Adding it as an argument to `emitCheckedGetOptionalValueFrom` makes more sense to me. Everywhere we call it in SILGen, we ought to be able to walk back to an AST formation that's either working on an explicit or implicit unwrapping. If not, it's probably reasonable to default to the "explicit" case.

[beccadax]

Fixed in #17826