Refine parameter type 'inout' diagnostics #9147
Conversation
|
@swift-ci please smoke test |
| } | ||
|
|
||
| // expected-error@+1 {{expression type '(inout MutableSubscripts) -> ()' is ambiguous without more context}} | ||
| let closure = { val in val.x = 7 } as (inout MutableSubscripts) -> () |
There was a problem hiding this comment.
Because the parameter is assumed immutable unless the user actually writes inout or var - supposedly (see FIXME below). This one usually manifests itself with anonymous arguments that need to be converted to named arguments because we don't like inferring inout on closure parameters. @xedin was looking into this at one point IIRC
There was a problem hiding this comment.
Right, we infer it inconsistently now, which is terrible, and then give a bad diagnostic in the non-inferred cases, which @CodaFi has improved. There are a handful of bugs about this.
| @@ -491,6 +491,9 @@ enum TypeResolutionFlags : unsigned { | |||
|
|
|||
| /// Whether we are checking the underlying type of a typealias. | |||
| TR_TypeAliasUnderlyingType = 0x4000000, | |||
|
|
|||
| /// Whether we are checking the parameter list of a subscript. | |||
| TR_SubscriptParameters = 0x8000000, | |||
There was a problem hiding this comment.
You could use this to resolve property types also, and remove the non-materializable check there.
|
Looks good, I'm a little concerned about the proliferation of TR_ flags we've been experiencing lately but I don't really have a good solution to that. |
|
I really want to find time to completely rewrite this file... In the meantime I'll look into the non-materializable checks. |
|
In the constraint solver, we use the notion of a locator to make context-sensitive decisions, among other things. I wonder if instead of TR_ flags we could pass in something like a 'TypeReprLocator'. Type resolution could then reach into the locator to ask questions about what kind of decl it's resolving, if its a top-level TypeRepr or nested inside of another type constructor, etc. Might be cleaner than passing around various flags and remembering to set them. |
lib/Sema/TypeCheckType.cpp
Outdated
| // inout is only valid for (non-subscript) function parameters. | ||
| if (options.contains(TR_SubscriptParameters) || | ||
| (!(options & TR_FunctionInput) && | ||
| !(options & TR_ImmediateFunctionInput))) { |
There was a problem hiding this comment.
Weird mix of contains and & here.
CodaFi
force-pushed
the
mutatis-mutandis
branch
2 times, most recently
from
f0fcaf9 to
a397ba6
Compare
- Subscripts parameter lists may not contain inout arguments, but we were rejecting this at the call site. Teach the type checker to reject them during type resolution instead. - We assumed a syntactic check for inout/var parameters would suffice given that a parameter unified to an InoutType. However, closures passed to function parameters with inout parameters in their parameter lists can also cause this case to appear, and we would emit a SourceLoc-less diagnostic. Instead, do not attempt this recovery path if the user did not actually write ‘var’ or ‘inout’ on the parameter type.
|
@swift-ci please smoke test |
|
@slavapestov I tried to plumb things through and it wound up being a little more than I anticipated. I'll save simplifying this code path for another date and get these in now. ⛵️ |
Subscripts' parameter lists may not contain inout arguments, but we
were rejecting them at call sites. Teach the type checker to reject
them during type resolution instead.
We assumed a syntactic check for inout/var parameters would suffice
given that a parameter unified to an InOutType. However, closures
passed to function parameters with inout parameters in their parameter
lists can also cause this case to appear, and we would emit a
SourceLoc-less diagnostic. Instead, do not attempt this recovery path
if the user did not actually write ‘var’ or ‘inout’ on the parameter
type.
Resolves SR-4751, SR-4214, SR-3644, and SR-2354.