feat: dependent contextual inference #51511
Conversation
src/compiler/checker.ts
Outdated
| instantiateType( | ||
| contextualType.immediateBaseConstraint, | ||
| createTypeMapper([contextualType], [valueType]) | ||
| ); |
There was a problem hiding this comment.
This doesn't seem to replace the types in default parameters in a type alias, which is not what I would have expected. That is, extracting out Exclude<keyof Self, "k" | "t"> to a default type parameter K (as I did in #40439) makes the test fail even though it doesn't really change anything.
src/compiler/checker.ts
Outdated
| if (newContextualType.immediateBaseConstraint!.flags & TypeFlags.StructuredType) { | ||
| newContextualType.immediateBaseConstraint = resolveStructuredTypeMembers(newContextualType.immediateBaseConstraint as StructuredType); | ||
| } |
There was a problem hiding this comment.
I don't know why this is needed, but given it is, how do you resolve any kind of type and not just structured?
src/compiler/checker.ts
Outdated
| let nodeLinks = getNodeLinks(node); | ||
| nodeLinks.flags &= ~NodeCheckFlags.TypeChecked; | ||
| nodeLinks.flags &= ~NodeCheckFlags.ContextChecked; | ||
| nodeLinks.resolvedType = undefined; | ||
| nodeLinks.resolvedEnumType = undefined; | ||
| nodeLinks.resolvedSignature = undefined; | ||
| nodeLinks.resolvedSymbol = undefined; | ||
| nodeLinks.resolvedIndexInfo = undefined; | ||
| nodeLinks.contextFreeType = undefined |
There was a problem hiding this comment.
I don't know if I'm missing something here or I have something extra here, basically I want to invalidate the previous check.
src/compiler/checker.ts
Outdated
| contextualType.immediateBaseConstraint.aliasTypeArguments && | ||
| contextualType.immediateBaseConstraint.aliasTypeArguments.length === 1 && | ||
| contextualType.immediateBaseConstraint.aliasTypeArguments[0].id === contextualType.id; |
There was a problem hiding this comment.
How do you check if type A is dependent on type B? That is I want to write doesTypeDependOn(contextualType.immediateBaseConstraint, contextualType).
There was a problem hiding this comment.
Perhaps you want to use isTypeParameterPossiblyReferenced or a similar thing?
There was a problem hiding this comment.
That probably requires the node to be checked (which we haven't yet), but even it didn't we want something like (a: Type, b: TypeParameter) => boolean
There was a problem hiding this comment.
Actually I realised not only it would be expensive but also I don't need a function that look ups the type tree of A and finds if it has B, I want a function that looks up the syntax tree of A's declaration and see if there is a reference to B. So I want something roughly like...
const isContextualTypeDependent =
contextualType &&
(contextualType & TypeFlags.TypeParameter) &&
contextualType.symbol.declarations[0].constraint &&
!!forEachChildRecursively(contextualType.symbol.declarations[0].constraint, node => {
if (
node.kind === SyntaxKind.TypeReference &&
getSymbolOfNode(node.typeName).id ===
getSymbolOfNode(contextualType.symbol.declarations[0]).id
) {
return true
}
})
|
|
||
| type F<T> = | ||
| { a: unknown | ||
| , b: (a: T["a" & keyof T]) => unknown |
There was a problem hiding this comment.
This test fails if I write T extends { a: infer X } ? X : never (ie as it's written in #51377) instead of T["a" & keyof T], which is something I would have not expected as both versions essentially do the same thing.
|
Just curious: @typescript-bot perf test this faster |
|
Heya @RyanCavanaugh, I've started to run the abridged perf test suite on this PR at 9d516b6. You can monitor the build here. Update: The results are in! |
|
@RyanCavanaugh Here they are:Comparison Report - main..51511
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Btw we can make it even more performant by bailing to default checking even more earlier by first checking for |
|
I was waiting for a green light as I said but I got bored :P so here are some advancements... Previously only As I said in some cases two passes won't suffice. So we keep doing checks till we yield the same type as previous check, or we hit the limit of maximum 3 checks. If we hit the limit then we produce a error that says "Dependent contextual inference requires too many passes and possibly infinite". And perhaps this should be a warning instead of an error. I've updated the todos in the description. |
| ((node as TypeReferenceNode).typeName as Identifier).escapedText === | ||
| (contextualType.symbol.declarations![0] as TypeParameterDeclaration).name.escapedText |
There was a problem hiding this comment.
This is probably a brittle way of writing this (unless I'm mistaken) but I don't know any other way that works. I would have used symbols but it seems that they aren't declared at this point.
| @@ -28417,6 +28417,96 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
| } | |||
|
|
|||
| function checkObjectLiteral(node: ObjectLiteralExpression, checkMode?: CheckMode): Type { | |||
| const contextualType = getContextualType(node, /*contextFlags*/ undefined); | |||
| const isContextualTypeDependent = | |||
There was a problem hiding this comment.
Have you tried using couldContainTypeVariables? A comment for this function mentions this:
// Return true if the given type could possibly reference a type parameter for which
// we perform type inference (i.e. a type parameter of a generic function). We cache
// results for union and intersection types for performance reasons.
It sounds like maybe you could reuse it
There was a problem hiding this comment.
No because here we don't want to check if X has any type parameter, we want to check if X has a T type parameter. So using isContextualTypeDependent = couldContainTypeVariable(contextualType.immediateConstraint) would make it true for even T extends F<U> (U being some other type parameter) when it should have been false (as it's not T extends F<T>).
There was a problem hiding this comment.
Right, but then perhaps that function could be extended with an optional argument, and this way you could limit the results to be based on the specific type param.
There was a problem hiding this comment.
I thought about that but I think that's quite non-trivial because it's implemented keeping in mind that we're checking against any type parameter... So for example caching is a big part of this, right now because we're checking for any type parameter the cache is implemented simply as a CouldContainTypeVariables flag, but if we want to cache for couldContainTypeVariables(type, typeParameters) or even couldContainTypeVariable(type, typeParameter) it would require a lot of ceremony.
Edit: I think you could implement couldContainTypeVariable(type, typeParameter)'s cache by simply memoizing via ids of the arguments, so it's not much of a ceremony, but I'm not convinced I want to change this already existing abstraction for my use-case. Might reconsider in future.
There was a problem hiding this comment.
Btw I realised the implementation is seriouly true to it's name "couldContainTypeVariable" and not "containsTypeVariable". This function doesn't definitively checks if the type as a type parameter, it just kinda bets, for example if it's an anonymous object type it just bets it has a type parameter and doesn't check furthur inside it. So if it returns true that means the type either has a type parameter or doesn't have a type parameter, but if it return false then it the type definately doesn't have a type parameter. So one can write bailouts like if (!couldContainTypeVariable(type)) return can gain some optimization and still not give bad results.
So there's no way I can use this function here as I want a definitive result. And extending it's implementation to give a definitive result might not be trivial and definitately more complex and slow than the ast check I'm doing here.
Fun fact: While working on #52088 I overlooked the "could" in it's name knowing perfectly what it means and relied on it which produced bad results, finally when I looked at the implementation I saw that it really takes the "could" in it's name seriously :P
Fixes #51377
Fixes #40439
This PR adds support for what I'm calling "dependent contextual inference"... Sometimes the type for a value depends on the value itself (see linked issues), in those cases the contextual type of the value becomes something like
T extends F<T>. Here what you'd want to do is first check the value against the contextual typeT extends F<T>(which is what already happens), let's say the type of the value was inferrred to beX, and then (this is the new part) you do a second check against the contextual typeT extends F<X>and the result of that would be the final contextually inferred type of the valueConsider this PR a shabby experiment, I want to first get a green light if we want to do something like this, if yes then I can work on it further.
If ya'll prefer me opening an issue proposing this change and discussing it there first then let me know. Types like
T extends F<T>pop up when you're typing DSLs and it'd be really great if we could support them better.Also note that most of this is a sophisticated guesswork, I knew what I wanted to do but I don't know much about the codebase or compilers xD, so there might be better ways to write it (I'll leave some review comments).
Also leaving some todos:
T extends <any-expression-that-has-T>, not justT extends F<T>. And perhaps even<any-expression-that-has-T>, where T extends <any-other-expression-that-has-T>.T extends <any-expression-that-has-T><any-expression-that-has-T>, where T extends <any-other-expression-that-has-T>Edit: I'm working on this further but first only view the first commit to get a rough enough idea of what's happening.