internal: don't instantiate concepts when lifting types #861
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary ======= Don't instantiate invocations of generic concepts when lifting types (`liftParamType`). Instead, only turn the invocation into a `tyUserTypeClassInst` type. This removes a usage of `semtypinst`'s "meta-types allowed" mode, and is thus a step towards removing the latter. Details ======= Instead of using `instGenericContainer(allowedMetaTypes = true)` for lifting the invocation of generic concepts into a `tyUserTypeClassInst`, the invocation is now copied, transitioned to a `tyUserTypeClassInst`, and then has the underlying `tyUserTypeClass` the invocation applies to added as the last element. The above is **not** the same as what `instGenericContainer` did. Most importantly, generic invocations used as arguments to the invocation are not instantiated. In practice, this means that a parameter type like `Concept[Generic[T]]` will reach `typeRel` and subsequently `matchUserTypeClass` as `(UserTypeClassInst (...) (GenericInvocation (...) (...)) (...))` instead of as `(UserTypeClassInst (...) (GenericInst (...) (...) (...)) (...))` The `tyGenericInvocation` has the `tfHasMeta` flag included and a `tyInferred` is thus later created when matching against the concept, which is correct. Since `typeRel` does support resolving `tyInferred` types that have a `tyGenericInvocation` as the base, this doesn't cause problems.
zerbina
added
compiler/sem
5 tasks
saem
approved these changes
Aug 28, 2023
There was a problem hiding this comment.
Hopefully I'm following along correctly, but given the change, if we're doing a sigmatch, then we're deferring instantiation until we're actually comparing an argument and parameter. This seems entirely correct, as we're not losing information/making the decision too quickly, as we're aiming for "late static binding".
|
/merge |
|
Merge requested by: @saem Contents after the first section break of the PR description has been removed and preserved below: |
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Don't instantiate invocations of generic concepts when lifting types
(
liftParamType). Instead, only turn the invocation into atyUserTypeClassInsttype.This removes a usage of
semtypinst's "meta-types allowed" mode, and isthus a step towards removing the latter.
Details
Instead of using
instGenericContainer(allowedMetaTypes = true)forlifting the invocation of generic concepts into a
tyUserTypeClassInst,the invocation is now copied, transitioned to a
tyUserTypeClassInst,and then has the underlying
tyUserTypeClassthe invocation applies toadded as the last element.
The above is not the same as what
instGenericContainerdid. Mostimportantly, generic invocations used as arguments to the invocation
are not instantiated. In practice, this means that a parameter type like
Concept[Generic[T]]will reachtypeReland subsequentlymatchUserTypeClassas(UserTypeClassInst (...) (GenericInvocation (...) (...)) (...))instead of as
(UserTypeClassInst (...) (GenericInst (...) (...) (...)) (...))The
tyGenericInvocationhas thetfHasMetaflag included and atyInferredis thus later created when matching against the concept,which is correct. Since
typeReldoes support resolvingtyInferredtypes that have a
tyGenericInvocationas the base, this doesn't causeproblems.