Moving IDE code to GenericSignature and GenericFunctionType #4849
Conversation
|
@swift-ci Please test |
|
Build failed |
|
Build failed |
|
@slavapestov awesome! Thanks for simplifying the ide part. |
|
@nkcsgexi Any ideas about the IDE/complete_constructor failure? When I run it locally, I see that if I run the test in isolation, it passes, but if I run it with the other IDE tests, the code completion cache somehow gets 'polluted', so when the HAVE_COMMA_1 query is performed, we have constructors in the results that we didn't expect. I don't think my changes affect which decls are chosen in the results, perhaps the existing completion cache code has a bug where it sometimes encounters cache collisions of this form? |
|
I cannot see any of your changes make the completion selection different. Have you tried to erase cache before running the tests (even though I think this is the default)? is it still failing? |
|
Yes, it's failing in ci. Every test run starts with a fresh completion cache, but any tests that appear before complete_constructor seem to affect the cache. |
slavapestov
force-pushed
the
ide-cleanup
branch
from
34eac65 to
0db454b
Compare
|
@swift-ci Please test |
|
Build failed |
|
@swift-ci Please clean test Linux |
|
Build failed |
|
@swift-ci Please clean test macOS |
|
Build failed |
For code completion to use Type::subst() without undoing
previously-added heuristics, we need a form of member lookup
that strips the outermost typealias type. For example, if we
have
protocol P {
associatedtype Fruit
}
struct Apple {}
typealias Pear = Apple
class Bowl : P {
typealias Fruit = Pear
}
With DesugarMemberTypes OFF, the substitution 'T := Bowl'
applied to 'T.Fruit' yields 'Bowl.Fruit'.
With DesugarMemberTypes ON, the same substitution instead
produces 'Pear' (note: not 'Apple'; we only desugar one
level here, to match the current code completion behavior).
Also, add another tweak that will become important shortly;
if a type witness cannot be derived, set the type witness
to an alias type pointing to an error type, instead of
directly storing an error type in there.
This fixes a source of non-determinism. The IDE/complete_constructor test would sometimes fail depending on the order in which prior tests ran, since those prior tests might populate the code completion cache.
There was a ton of complicated logic here to work around two problems: - Same-type constraints were not represented properly in RequirementReprs, requiring us to store them in strong form and parse them out when printing type interfaces. - The TypeBase::getAllGenericArgs() method did not do the right thing for members of protocols and protocol extensions, and so instead of simple calls to Type::subst(), we had an elaborate 'ArchetypeTransformer' abstraction repeated in two places. Rewrite this code to use GenericSignatures and GenericFunctionType instead of old-school GenericParamLists and PolymorphicFunctionType. This changes the code completion and AST printer output slightly. A few of the changes are actually fixes for cases where the old code didn't handle substitutions properly. A few others are subjective, for example a generic parameter list of the form <T : Proto> now prints as <T where T : Proto>. We can add heuristics to make the output whatever we want here; the important thing is that now we're using modern abstractions.
slavapestov
force-pushed
the
ide-cleanup
branch
from
0db454b to
2c88c75
Compare
|
@swift-ci Please test |
|
Build failed |
|
@swift-ci Please smoke test |
1 similar comment
|
@swift-ci Please smoke test |
This was almost identical to getMemberSubstitutions(), except protocol and protocol extension members are not properly supported. Now that this method is no longer used by the ASTPrinter, there are only two remaining usages, both trivially refactorable to use better APIs.
RequirementReprs stored serialized references to archetypes, which do not have enough information to reconstruct same-type requirements. For this reason, we would serialize the 'as written' requirement string as well as the actual types, which is a horrible hack. Now that the ASTPrinter and SourceKit use GenericSignatures, none of this is needed anymore.
slavapestov
force-pushed
the
ide-cleanup
branch
from
2c88c75 to
581c99b
Compare
|
@swift-ci Please smoke test |
1 similar comment
|
@swift-ci Please smoke test |
|
This is fantastic. Merge it! ;) |
This cleans up code completion, module interface printing, and the SourceKit CursorInfo thing to stop using
PolymorphicFunctionTypeas well as some obsolete functionality on aGenericParamList.The old code contained a number of workarounds for limitations of the type-checked
RequirementReprrepresentation, which should no longer be used in favor ofGenericSignature::getRequirements(). The new code for printingGenericSignaturesandGenericFunctionTypesis a lot simpler in comparison. Everything is written with interface types, and requirements are canonicalized and easily substitutable.This means we don't have to serialize the requirements section of a
GenericParamListanymore at all -- previously both the types and string representations of requirements had to be serialized, and the strings later parsed, which was gross.The parameters of a
GenericParamListare still serialized, to give a home toGenericTypeParamDeclsthat appear in sugaredGenericTypeParamTypes, which makes sense I think.There were some minor changes to code completion and interface printing output. I hope that's okay. We should just add whatever heuristics we need to the new GenericSignature-based printing instead.