Emit a warning when optionals are coerced to Any. #4900
Conversation
Emit a warning for optionals that are implicitly converted to Any, and add fixits giving options to: - Add '??' with a default value after - Force-unwrap the optional with '!' - Explicitly cast to 'as Any' to silence the warning This covers diagnostics aspect of SE-0140. rdar://problem/28196843
|
@swift-ci Test OS X. |
|
@DougGregor Can you review for swift-3.0-branch? |
| auto subExpr = cast<ErasureExpr>(E)->getSubExpr(); | ||
| auto erasedTy = subExpr->getType()->getDesugaredType(); | ||
| if (erasedTy->getOptionalObjectType()) { | ||
| TC.diagnose(subExpr->getStartLoc(), diag::optional_to_any_coercion, |
There was a problem hiding this comment.
I don't think we should be eagerly desugaring here; it makes the diagnostic worse. getOptionalObjectType should look through sugar already.
There was a problem hiding this comment.
(as should isAny and any other such query)
There was a problem hiding this comment.
Ah yes, that's the type we're displaying.
I'll commit a follow-up to master and then cherry-pick and add that to this PR.
| NOTE(force_optional_to_any,none, | ||
| "force-unwrap the value to avoid the warning", ()) | ||
| NOTE(silence_optional_to_any,none, | ||
| "explicitly cast to Any with 'as Any' to silence the warning", ()) |
There was a problem hiding this comment.
Nitpick: Clang typically says "this warning" rather than "the warning". I don't think Swift has enough of these to count as having a convention.
|
Do we want this warning for |
|
You can't put an optional into an |
|
Ah, I didn't realize that. Okay, thanks. |
|
@swift-ci Test OS X platform. |
|
@swift-ci Test OS X. |
|
@jrose-apple Thanks, I think everything is fixed now. |
|
@DougGregor Doug, I think this is ready for review now (although tests are still running at the moment). |
| let g: Any = (x) as (Any) | ||
|
|
||
| _ = takeAny(f as? Int, g) // expected-warning {{expression implicitly coerced from 'Int?' to Any}} | ||
| // expected-note@-1 {{provide a default value to avoid this warning}} |
There was a problem hiding this comment.
I'd like at least one of these note-cascades to test the Fix-Its themselves using the -verify syntax, e.g.,
// expected-note@-1 {{provide a default value to avoid this warning}}{{23-23= ?? <#default value#>}}
to make sure the locations are reasonable and don't break in the future.
There was a problem hiding this comment.
Yes, you're right! I'll add one on master.
Pulling this diagnostic change for SE-0140 into swift-3.0-branch.
Emit a warning for optionals that are implicitly converted to Any, and
add fixits giving options to:
This covers diagnostics aspect of SE-0140.
rdar://problem/28196843