[SE-309] CSDiag: Add a fix-it that replaces an existential parameter type with its generic counterpart #41198
Conversation
AnthonyLatsis
force-pushed
the
se-309-fixit
branch
from
24ab9f8 to
ca19d1c
Compare
AnthonyLatsis
changed the title
AnthonyLatsis
force-pushed
the
se-309-fixit
branch
from
ca19d1c to
0a82566
Compare
lib/Sema/CSDiagnostics.cpp
Outdated
| if (isa<InOutTypeRepr>(PD->getTypeRepr())) | ||
| return true; | ||
|
|
||
| constexpr StringRef GPNamePlaceholder = "<#generic parameter#>"; |
There was a problem hiding this comment.
Unfortunately, placeholders cannot be correlated and compile to errors rather than wildcards, which may take inexperienced users by surprise. We could use T when there is no generic signature, and something neutral otherwise like GenericParameter with some number suffix for disambiguation, but that can still be abused in many ways. Or we could fall back to placeholders, now that a generic signature gives us some degree of confidence that the user is familiar with generics.
There was a problem hiding this comment.
I think this is fine, we shouldn't attempt to name the parameter here and let users do it instead, maybe add name after generic parameter to make it more explicit...
|
Given that the SE-0341 review is going well-ish, should we inject |
We could also provide both options to honor progressive disclosure; a sudden |
AnthonyLatsis
force-pushed
the
se-309-fixit
branch
3 times, most recently
from
5e6a8b0 to
f6f1ea9
Compare
|
@xedin ping |
|
@AnthonyLatsis Thanks for the ping! Should be able to take a look at this a bit later today. |
There was a problem hiding this comment.
LGTM! My only concern is that multiple expressions are going to produce these fix-its and that would create chaos if fix-its where auto applied... Could you please, as a follow-up, explore whether it would be possible to adjust DiagnosticEngine to track whether exactly the same fix-it has been already applied to a particular source region before and just drop it?
lib/Sema/CSDiagnostics.cpp
Outdated
| if (isa<InOutTypeRepr>(PD->getTypeRepr())) | ||
| return true; | ||
|
|
||
| constexpr StringRef GPNamePlaceholder = "<#generic parameter#>"; |
There was a problem hiding this comment.
I think this is fine, we shouldn't attempt to name the parameter here and let users do it instead, maybe add name after generic parameter to make it more explicit...
|
@xedin Thank you for the feedback! Do you have an opinion on Doug's suggestion to inject |
|
@AnthonyLatsis I'm not sure, that's why I didn't comment on that. Maybe it makes sense to implement it under the the same flag that SE-0341 is using and make a decision once that flag gets removed... |
AnthonyLatsis
force-pushed
the
se-309-fixit
branch
from
f6f1ea9 to
ef9a2ee
Compare
|
@swift-ci please smoke test |
…type with its generic equivalent
AnthonyLatsis
force-pushed
the
se-309-fixit
branch
from
ef9a2ee to
2342712
Compare
|
@swift-ci please smoke test |
AnthonyLatsis
changed the title
If the base of an unsupported existential member access is a reference to a function/subscript parameter, offer a fix-it that replaces the existential parameter type with its generic counterpart, e.g.
func foo(p: any P) → func foo<T>(p: T).