Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Optimize substitution types #50397

Merged
merged 6 commits into from
Aug 26, 2022
Merged

Optimize substitution types #50397

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c09b7dc
Optimize substitution type infrastructure
ahejlsberg Aug 22, 2022
a4a9ede
Accept new baselines
ahejlsberg Aug 22, 2022
e73d268
Preserve instantiated substitution types for type variables
ahejlsberg Aug 22, 2022
f48a923
Restrictive type parameters should have no constraint
ahejlsberg Aug 22, 2022
95ea0ba
Fix issues from top100 test run
ahejlsberg Aug 25, 2022
aed9659
Accept new baselines
ahejlsberg Aug 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 33 additions & 22 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12344,7 +12344,7 @@ namespace ts {
return constraint && getBaseConstraint(constraint);
}
if (t.flags & TypeFlags.Substitution) {
return getBaseConstraint((t as SubstitutionType).substitute);
return getBaseConstraint(getSubstitutionIntersection(t as SubstitutionType));
amcasey marked this conversation as resolved.
Show resolved Hide resolved
}
return t;
}
Expand Down Expand Up @@ -13881,22 +13881,28 @@ namespace ts {
return links.resolvedJSDocType;
}

function getSubstitutionType(baseType: Type, substitute: Type) {
if (substitute.flags & TypeFlags.AnyOrUnknown || substitute === baseType) {
function getSubstitutionType(baseType: Type, constraint: Type) {
if (constraint.flags & TypeFlags.AnyOrUnknown || constraint === baseType ||
Copy link
Member

@DanielRosenwasser DanielRosenwasser Aug 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

constraint === baseType

Is this just when someone writes

T extends T ? TrueType<T> : FalseType<T>

?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I presume so. The check was already there.

baseType.flags & TypeFlags.Substitution && (baseType as SubstitutionType).constraint === constraint ||
!isGenericType(baseType) && !isGenericType(constraint)) {
return baseType;
}
const id = `${getTypeId(baseType)}>${getTypeId(substitute)}`;
const id = `${getTypeId(baseType)}>${getTypeId(constraint)}`;
const cached = substitutionTypes.get(id);
if (cached) {
return cached;
}
const result = createType(TypeFlags.Substitution) as SubstitutionType;
result.baseType = baseType;
result.substitute = substitute;
result.constraint = constraint;
substitutionTypes.set(id, result);
return result;
}

function getSubstitutionIntersection(substitutionType: SubstitutionType) {
return getIntersectionType([substitutionType.constraint, substitutionType.baseType]);
}

function isUnaryTupleTypeNode(node: TypeNode) {
return node.kind === SyntaxKind.TupleType && (node as TupleTypeNode).elements.length === 1;
}
Expand Down Expand Up @@ -13941,7 +13947,7 @@ namespace ts {
}
node = parent;
}
return constraints ? getSubstitutionType(type, getIntersectionType(append(constraints, type))) : type;
return constraints ? getSubstitutionType(type, getIntersectionType(constraints)) : type;
}

function isJSDocTypeReference(node: Node): node is TypeReferenceNode {
Expand Down Expand Up @@ -15341,7 +15347,7 @@ namespace ts {
type.flags & TypeFlags.Conditional ? (type as ConditionalType).root.isDistributive && (type as ConditionalType).checkType === typeVariable :
type.flags & (TypeFlags.UnionOrIntersection | TypeFlags.TemplateLiteral) ? every((type as UnionOrIntersectionType | TemplateLiteralType).types, isDistributive) :
type.flags & TypeFlags.IndexedAccess ? isDistributive((type as IndexedAccessType).objectType) && isDistributive((type as IndexedAccessType).indexType) :
type.flags & TypeFlags.Substitution ? isDistributive((type as SubstitutionType).substitute) :
type.flags & TypeFlags.Substitution ? isDistributive((type as SubstitutionType).baseType) && isDistributive((type as SubstitutionType).constraint):
type.flags & TypeFlags.StringMapping ? isDistributive((type as StringMappingType).type) :
false;
}
Expand Down Expand Up @@ -15861,7 +15867,7 @@ namespace ts {
if (type.flags & TypeFlags.Substitution) {
if (!((type as SubstitutionType).objectFlags & ObjectFlags.IsGenericTypeComputed)) {
(type as SubstitutionType).objectFlags |= ObjectFlags.IsGenericTypeComputed |
getGenericObjectFlags((type as SubstitutionType).substitute) | getGenericObjectFlags((type as SubstitutionType).baseType);
getGenericObjectFlags((type as SubstitutionType).baseType) | getGenericObjectFlags((type as SubstitutionType).constraint);
}
return (type as SubstitutionType).objectFlags & ObjectFlags.IsGenericType;
}
Expand Down Expand Up @@ -17407,17 +17413,18 @@ namespace ts {
return getConditionalTypeInstantiation(type as ConditionalType, combineTypeMappers((type as ConditionalType).mapper, mapper), aliasSymbol, aliasTypeArguments);
}
if (flags & TypeFlags.Substitution) {
const maybeVariable = instantiateType((type as SubstitutionType).baseType, mapper);
if (maybeVariable.flags & TypeFlags.TypeVariable) {
return getSubstitutionType(maybeVariable as TypeVariable, instantiateType((type as SubstitutionType).substitute, mapper));
}
else {
const sub = instantiateType((type as SubstitutionType).substitute, mapper);
if (sub.flags & TypeFlags.AnyOrUnknown || isTypeAssignableTo(getRestrictiveInstantiation(maybeVariable), getRestrictiveInstantiation(sub))) {
return maybeVariable;
}
return sub;
}
const newBaseType = instantiateType((type as SubstitutionType).baseType, mapper);
const newConstraint = instantiateType((type as SubstitutionType).constraint, mapper);
// A substitution type originates in the true branch of a conditional type and can be resolved
// to just the base type in the same cases as the conditional type resolves to its true branch
// (because the base type is then known to satisfy the constraint).
if (!(newBaseType.flags & TypeFlags.TypeVariable) && (
newConstraint.flags & TypeFlags.AnyOrUnknown ||
!isGenericType(newBaseType) && !isGenericType(newConstraint) ||
isTypeAssignableTo(getRestrictiveInstantiation(newBaseType), getRestrictiveInstantiation(newConstraint)))) {
return newBaseType;
}
return getSubstitutionType(newBaseType, newConstraint);
}
return type;
}
Expand Down Expand Up @@ -18456,7 +18463,7 @@ namespace ts {
const t = isFreshLiteralType(type) ? (type as FreshableType).regularType :
getObjectFlags(type) & ObjectFlags.Reference ? (type as TypeReference).node ? createTypeReference((type as TypeReference).target, getTypeArguments(type as TypeReference)) : getSingleBaseForNonAugmentingSubtype(type) || type :
type.flags & TypeFlags.UnionOrIntersection ? getNormalizedUnionOrIntersectionType(type as UnionOrIntersectionType, writing) :
type.flags & TypeFlags.Substitution ? writing ? (type as SubstitutionType).baseType : (type as SubstitutionType).substitute :
type.flags & TypeFlags.Substitution ? writing ? (type as SubstitutionType).baseType : getSubstitutionIntersection(type as SubstitutionType) :
type.flags & TypeFlags.Simplifiable ? getSimplifiedType(type, writing) :
type;
if (t === type) return t;
Expand Down Expand Up @@ -19539,7 +19546,11 @@ namespace ts {
}
}
if (sourceFlags & TypeFlags.Substitution) {
return isRelatedTo((source as SubstitutionType).substitute, (target as SubstitutionType).substitute, RecursionFlags.Both, /*reportErrors*/ false);
if (result = isRelatedTo((source as SubstitutionType).baseType, (target as SubstitutionType).baseType, RecursionFlags.Both, /*reportErrors*/ false)) {
if (result &= isRelatedTo((source as SubstitutionType).constraint, (target as SubstitutionType).constraint, RecursionFlags.Both, /*reportErrors*/ false)) {
return result;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a meaningful break in behavior, right? Previously type FilterNever<T> = T extends never ? T : never would have allowed FilterNever<A> to be assignable to FilterNever<B> (where A and B are both type parameters), since T & never is just never, and eliminated the need to relate the type parameter part, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in the code path for identityRelation, so hardly matters. In your particular example the types wouldn't relate anyway because the type parameters aren't identical.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what would technically make it a break, right? Previously this related substitutions only, so A sub A & never and B sub B & never both looked identically like never?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's true. But they definitely shouldn't be related, so I guess this fixes a bug as well.

}
}
}
if (!(sourceFlags & TypeFlags.Object)) {
return Ternary.False;
Expand Down Expand Up @@ -22677,7 +22688,7 @@ namespace ts {
}
else if (source.flags & TypeFlags.Substitution) {
inferFromTypes((source as SubstitutionType).baseType, target);
inferWithPriority((source as SubstitutionType).substitute, target, InferencePriority.SubstituteSource); // Make substitute inference at a lower priority
inferWithPriority(getSubstitutionIntersection(source as SubstitutionType), target, InferencePriority.SubstituteSource); // Make substitute inference at a lower priority
}
else if (target.flags & TypeFlags.Conditional) {
invokeOnce(source, target, inferToConditionalType);
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/tracing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ namespace ts { // eslint-disable-line one-namespace-per-file
const substitutionType = type as SubstitutionType;
substitutionProperties = {
substitutionBaseType: substitutionType.baseType?.id,
substituteType: substitutionType.substitute?.id,
constraintType: substitutionType.constraint?.id,
amcasey marked this conversation as resolved.
Show resolved Hide resolved
};
}

Expand Down
4 changes: 2 additions & 2 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6077,8 +6077,8 @@ namespace ts {
// types disappear upon instantiation (just like type parameters).
export interface SubstitutionType extends InstantiableType {
objectFlags: ObjectFlags;
baseType: Type; // Target type
substitute: Type; // Type to substitute for type parameter
baseType: Type; // Target type
constraint: Type; // Constraint that target type is known to satisfy
}

/* @internal */
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/api/tsserverlibrary.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2832,7 +2832,7 @@ declare namespace ts {
export interface SubstitutionType extends InstantiableType {
objectFlags: ObjectFlags;
baseType: Type;
substitute: Type;
constraint: Type;
}
export enum SignatureKind {
Call = 0,
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/api/typescript.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2832,7 +2832,7 @@ declare namespace ts {
export interface SubstitutionType extends InstantiableType {
objectFlags: ObjectFlags;
baseType: Type;
substitute: Type;
constraint: Type;
}
export enum SignatureKind {
Call = 0,
Expand Down
6 changes: 3 additions & 3 deletions tests/baselines/reference/callOfConditionalTypeWithConcreteBranches.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@ function fn2<T>(arg: Q2<T>) {
arg(arg => useT(arg));
>arg(arg => useT(arg)) : void
>arg : Q2<T>
>arg => useT(arg) : (arg: T & number) => void
>arg : T & number
>arg => useT(arg) : (arg: number) => void
>arg : number
>useT(arg) : void
>useT : (_arg: T) => void
>arg : T & number
>arg : number
}
// Legal invocations are not problematic
fn2<string | number>(m => m(42));
Expand Down
Loading