Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion lib/Sema/TypeCheckPattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1230,7 +1230,14 @@ Pattern *TypeChecker::coercePatternToType(
// If the whole pattern is implicit, the user didn't write it.
// Assume the compiler knows what it's doing.
} else if (diagTy->isEqual(Context.TheEmptyTupleType)) {
shouldRequireType = true;
// Async-let bindings are commonly used to run a Void-returning
// synchronous function in an async context. As a policy choice, don't
// diagnose a Void result on these bindings as potentially unexpected.
if (!isOptional && var->isAsyncLet()) {
shouldRequireType = false;
} else {
shouldRequireType = true;
}
} else if (auto MTT = diagTy->getAs<AnyMetatypeType>()) {
if (MTT->getInstanceType()->isAnyObject())
shouldRequireType = true;
Expand Down
31 changes: 31 additions & 0 deletions test/decl/var/async_let.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,34 @@ func testInterpolation() async {
async let y = "\(12345)"
_ = await y
}

// https://forums.swift.org/t/disable-constant-inferred-to-have-type-warning-when-using-async-let/83025

func testVoidResultTypeDiagnostics() async {
async let void = print("hello")
await void
async let void2 = ()
await void2
async let void3 = Void()
await void3
async let void4 = { _ = 42 }()
await void4

@Sendable func syncVoid() {}
async let void5 = syncVoid()
await void5

@Sendable func asyncVoid() async {}
async let void6 = asyncVoid()
await void6

// expected-warning @+2 {{constant 'maybeVoid' inferred to have type '()?', which may be unexpected}}
// expected-note @+1 {{add an explicit type annotation to silence this warning}}
async let maybeVoid = { Bool.random() ? () : nil }()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if Void? I'm not entirely sure it's worth diagnosing since this can be something that occurs for e.g optional chaining a call to a Void returning method, e.g x?.foo()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you feel like that logic ends at a single level of optionality?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes since optional chaining flattens nested optionals, I don't think a nested optional of Void is likely to be something that comes up very often in practice though, and I'm not sure how much value the warning would have in such cases (it's valuable for regular bindings because the binding itself is probably a mistake, but that's not necessarily the case for async let since the binding does more than provide a value). So I would be fine if we didn't add a special case for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @hamishknight – and just to be sure i'm following correctly: you think it'd be okay to drop the warning for any level of optionally-wrapped Void types on these bindings because 1 level may often be the result of optional chaining, and >1 level is probably uncommon enough to not warrant special handling. does that sound right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes exactly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

follow-up q: do you think the remaining 'unexpected type' checks currently implemented should also apply here, or are there some of those that you think would make sense to suppress as well?

await maybeVoid

// expected-warning @+2 {{constant 'boxOVoid' inferred to have type '[()]', which may be unexpected}}
// expected-note @+1 {{add an explicit type annotation to silence this warning}}
async let boxOVoid = { [(), (), ()] }()
await boxOVoid // expected-warning {{expression of type '[()]' is unused}}
}