8000 Use objects instead of closures for type mappers by ahejlsberg · Pull Request #36576 · microsoft/TypeScript · GitHub
[go: up one dir, main page]
Skip to content

Use objects instead of closures for type mappers #36576

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

Merged
merged 11 commits into from
Mar 11, 2020
Merged

Use objects instead of closures for type mappers #36576

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
63338f9
Use objects instead of closures for type mappers
ahejlsberg Feb 2, 2020
979bc6a
Flatten combined type mappers
ahejlsberg Feb 2, 2020
82cd7a1
Merge branch 'master' into typeMappersAsObjects
ahejlsberg Mar 5, 2020
8e6a2b1
Single point of creation for type mappers
ahejlsberg Mar 5, 2020
c7d3806
More optimizations
ahejlsberg Mar 5, 2020
163ba2f
Fix lint error
ahejlsberg Mar 5, 2020
61f0c7a
Fewer symbol instantiations / discard type mapper after instantiation
ahejlsberg Mar 5, 2020
50de2db
More optimizations
ahejlsberg Mar 6, 2020
ce9ddf3
Simplify mapper layout and cache in composite mappers
ahejlsberg Mar 8, 2020
30e7a18
Removing cache as it doesn't seem to matter much
ahejlsberg Mar 8, 2020
1cb54e1
Get rid of identityMapper
ahejlsberg Mar 10, 2020
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
Prev Previous commit
Next Next commit
Simplify mapper layout and cache in composite mappers
  • Loading branch information
@ahejlsberg
ahejlsberg committed Mar 8, 2020
commit ce9ddf3d2f6436953164995b0d54a9e044653254
84 changes: 38 additions & 46 deletions src/compiler/checker.ts
View file
Open in desktop
8000
Original file line number Diff line number Diff line change
Expand Up @@ -9588,8 +9588,7 @@ namespace ts {
const checkType = (<ConditionalType>type).checkType;
const constraint = getLowerBoundOfKeyType(checkType);
if (constraint !== checkType) {
const mapper = makeUnaryTypeMapper((<ConditionalType>type).root.checkType, constraint);
return getConditionalTypeInstantiation(<ConditionalType>type, combineTypeMappers(mapper, (<ConditionalType>type).mapper));
return getConditionalTypeInstantiation(<ConditionalType>type, prependTypeMapping((<ConditionalType>type).root.checkType, constraint, (<ConditionalType>type).mapper));
}
}
return type;
Expand Down Expand Up @@ -9639,7 +9638,7 @@ namespace ts {
// Create a mapper from T to the current iteration type constituent. Then, if the
// mapped type is itself an instantiated type, combine the iteration mapper with the
// instantiation mapper.
const templateMapper = addTypeMapping(type.mapper, typeParameter, t);
const templateMapper = appendTypeMapping(type.mapper, typeParameter, t);
// If the current iteration type constituent is a string literal type, create a property.
// Otherwise, for type string create a string index signature.
if (isTypeUsableAsPropertyName(t)) {
Expand Down Expand Up @@ -9943,8 +9942,7 @@ namespace ts {
const simplified = getSimplifiedType(type.checkType, /*writing*/ false);
const constraint = simplified === type.checkType ? getConstraintOfType(simplified) : simplified;
if (constraint && constraint !== type.checkType) {
const mapper = makeUnaryTypeMapper(type.root.checkType, constraint);
const instantiated = getConditionalTypeInstantiation(type, combineTypeMappers(mapper, type.mapper));
const instantiated = getConditionalTypeInstantiation(type, prependTypeMapping(type.root.checkType, constraint, type.mapper));
if (!(instantiated.flags & TypeFlags.Never)) {
return instantiated;
}
Expand Down Expand Up @@ -10175,8 +10173,7 @@ namespace ts {
if (typeVariable) {
const constraint = getConstraintOfTypeParameter(typeVariable);
if (constraint && (isArrayType(constraint) || isTupleType(constraint))) {
const mapper = makeUnaryTypeMapper(typeVariable, constraint);
return instantiateType(type, combineTypeMappers(mapper, type.mapper));
return instantiateType(type, prependTypeMapping(typeVariable, constraint, type.mapper));
}
}
return type;
Expand Down Expand Up @@ -12903,7 +12900,7 @@ namespace ts {
8000 // types rules (i.e. proper contravariance) for inferences.
inferTypes(context.inferences, checkType, extendsType, InferencePriority.NoConstraints | InferencePriority.AlwaysStrict);
}
combinedMapper = combineTypeMappers(mapper, context.mapper);
combinedMapper = mergeTypeMappers(mapper, context.mapper);
}
// Instantiate the extends type including inferences for 'infer T' type parameters
const inferredExtendsType = combinedMapper ? instantiateType(root.extendsType, combinedMapper) : extendsType;
Expand Down Expand Up @@ -13543,37 +13540,39 @@ namespace ts {
}

function createTypeMapper(sources: readonly TypeParameter[], targets: readonly Type[] | undefined): TypeMapper {
return sources.length === 1 ? makeUnaryTypeMapper(sources[0], targets ? targets[0] : anyType) :
sources.length === 2 ? makeSimpleTypeMapper(sources[0], targets ? targets[0] : anyType, sources[1], targets ? targets[1] : anyType) :
makeArrayTypeMapper(sources, targets);
return sources.length === 1 ? makeUnaryTypeMapper(sources[0], targets ? targets[0] : anyType) : makeArrayTypeMapper(sources, targets);
}

function getMappedType(type: Type, map: TypeMapper): Type {
switch (map.kind) {
function getMappedType(type: Type, mapper: TypeMapper): Type {
Copy link
Member

Choose a reason for hiding this comment

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

I kinda-sorta wanna preemptively make this a loop rather than a recursive function to better optimize the recursive cases, but... it's not strictly required.

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 looked at that, but just makes the code more complex for no appreciable gain.

switch (mapper.kind) {
case TypeMapKind.Simple:
return type === map.source1 ? map.target1 : type === map.source2 ? map.target2 : type;
return type === mapper.source ? mapper.target : type;
case TypeMapKind.Array:
const sources = map.sources;
const targets = map.targets;
const sources = mapper.sources;
const targets = mapper.targets;
for (let i = 0; i < sources.length; i++) {
if (type === sources[i]) {
return targets ? targets[i] : anyType;
}
}
return type;
case TypeMapKind.Function:
return map.func(type);
return mapper.func(type);
case TypeMapKind.Composite:
return instantiateType(getMappedType(type, map.mapper1), map.mapper2);
case TypeMapKind.Merged:
if (type === mapper.cachedSource) return mapper.cachedTarget;
const t1 = getMappedType(type, mapper.mapper1);
const t2 = t1 !== type && mapper.kind === TypeMapKind.Composite ? instantiateType(t1, mapper.mapper2) : getMappedType(t1, mapper.mapper2);
if (t2 !== type) {
mapper.cachedSource = type;
mapper.cachedTarget = t2;
}
return t2;
}
}

function makeUnaryTypeMapper(source: Type, target: Type): TypeMapper {
return makeSimpleTypeMapper(source, target, anyType, anyType);
}

function makeSimpleTypeMapper(source1: Type, target1: Type, source2: Type, target2: Type): TypeMapper {
return { kind: TypeMapKind.Simple, source1, target1, source2, target2 };
return { kind: TypeMapKind.Simple, source, target };
}

function makeArrayTypeMapper(sources: readonly TypeParameter[], targets: readonly Type[] | undefined): TypeMapper {
Expand All @@ -13584,8 +13583,8 @@ namespace ts {
return { kind: TypeMapKind.Function, func };
}

function makeCompositeTypeMapper(mapper1: TypeMapper, mapper2: TypeMapper): TypeMapper {
return { kind: TypeMapKind.Composite, mapper1, mapper2 };
function makeCompositeTypeMapper(kind: TypeMapKind.Composite | TypeMapKind.Merged, mapper1: TypeMapper, mapper2: TypeMapper): TypeMapper {
return { kind, mapper1, mapper2, cachedSource: undefined!, cachedTarget: undefined! };
}

function createTypeEraser(sources: readonly TypeParameter[]): TypeMapper {
Expand All @@ -13600,27 +13599,20 @@ namespace ts {
return makeFunctionTypeMapper(t => findIndex(context.inferences, info => info.typeParameter === t) >= index ? unknownType : t);
}

function combineTypeMappers(mapper1: TypeMapper | undefined, mapper2: TypeMapper): TypeMapper;
function combineTypeMappers(mapper1: TypeMapper, mapper2: TypeMapper | undefined): TypeMapper;
function combineTypeMappers(mapper1: TypeMapper, mapper2: TypeMapper): TypeMapper {
return !mapper1 ? mapper2 : !mapper2 ? mapper1 : makeCompositeTypeMapper(mapper1, mapper2);
function combineTypeMappers(mapper1: TypeMapper | undefined, mapper2: TypeMapper): TypeMapper {
return mapper1 ? makeCompositeTypeMapper(TypeMapKind.Composite, mapper1, mapper2) : mapper2;
}

function addTypeMapping(mapper: TypeMapper | undefined, source: TypeParameter, target: Type) {
return mapper && mapper.kind === TypeMapKind.Simple && mapper.source2 === mapper.target2 ?
makeSimpleTypeMapper(mapper.source1, mapper.target1, source, target) :
combineTypeMappers(mapper, makeUnaryTypeMapper(source, target));
function mergeTypeMappers(mapper1: TypeMapper | undefined, mapper2: TypeMapper): TypeMapper {
return mapper1 ? makeCompositeTypeMapper(TypeMapKind.Merged, mapper1, mapper2) : mapper2;
}

function createReplacementMapper(source: Type, target: Type, baseMapper: TypeMapper): TypeMapper {
switch (baseMapper.kind) {
case TypeMapKind.Simple:
return makeSimpleTypeMapper(baseMapper.source1, baseMapper.source1 === source ? target : baseMapper.target1,
baseMapper.source2, baseMapper.source2 === source ? target : baseMapper.target2);
case TypeMapKind.Array:
return makeArrayTypeMapper(baseMapper.sources, map(baseMapper.targets, (t, i) => baseMapper.sources[i] === source ? target : t));
}
return makeFunctionTypeMapper(t => t === source ? target : getMappedType(t, baseMapper));
function prependTypeMapping(source: Type, target: Type, mapper: TypeMapper | undefined) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm getting mixed up. When you combine two maps, you look in the first map first and then, if you find something, do you stop looking or apply the second map to the first target? From the handling of composite maps in getMappedType it seems like it might be the latter, but this appears to do the former for unary maps?

Copy link
Member Author

Choose a reason for hiding this comment

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

The particular test here is to see if the second source/target pair in the simple mapper is unused (recall, when source and target are the same, we have a no-op). If so, we create a new simple mapper with both source/target pairs in use.

One added twist with composite mappers is that the first mapper may map to some type that the second mapper further maps. For example, the first mapper might map from T to U[] and the second mapper from U to string. This also explains why we directly call the getMappedType with the first mapper, but then call instantiateType with the second one.

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 should add, in the addTypeMapping case, we actually don't care about the ability for the second mapping to affect the first mapping, which is why we can do the simple mapper optimization.

Copy link
Member

Choose a reason for hiding this comment

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

So maps are combined by composition, but we happen to know that this particular composition is equivalent to concatenation?

return !mapper ? makeUnaryTypeMapper(source, target) : makeCompositeTypeMapper(TypeMapKind.Merged, makeUnaryTypeMapper(source, target), mapper);
}

function appendTypeMapping(mapper: TypeMapper | undefined, source: Type, target: Type) {
return !mapper ? makeUnaryTypeMapper(source, target) : makeCompositeTypeMapper(TypeMapKind.Merged, mapper, makeUnaryTypeMapper(source, target));
}

function getRestrictiveTypeParameter(tp: TypeParameter) {
Expand Down Expand Up @@ -13815,7 +13807,7 @@ namespace ts {
if (typeVariable !== mappedTypeVariable) {
return mapType(getReducedType(mappedTypeVariable), t => {
if (t.flags & (TypeFlags.AnyOrUnknown | TypeFlags.InstantiableNonPrimitive | TypeFlags.Object | TypeFlags.Intersection) && t !== wildcardType && t !== errorType) {
const replacementMapper = createReplacementMapper(typeVariable, t, mapper);
const replacementMapper = prependTypeMapping(typeVariable, t, mapper);
return isArrayType(t) ? instantiateMappedArrayType(t, type, replacementMapper) :
isTupleType(t) ? instantiateMappedTupleType( A3E2 t, type, replacementMapper) :
instantiateAnonymousType(type, replacementMapper);
Expand Down Expand Up @@ -13851,7 +13843,7 @@ namespace ts {
}

function instantiateMappedTypeTemplate(type: MappedType, key: Type, isOptional: boolean, mapper: TypeMapper) {
const templateMapper = addTypeMapping(mapper, getTypeParameterFromMappedType(type), key);
const templateMapper = appendTypeMapping(mapper, getTypeParameterFromMappedType(type), key);
const propType = instantiateType(getTemplateTypeFromMappedType(<MappedType>type.target || type), templateMapper);
const modifiers = getMappedTypeModifiers(type);
return strictNullChecks && modifiers & MappedTypeModifiers.IncludeOptional && !maybeTypeOfKind(propType, TypeFlags.Undefined | TypeFlags.Void) ? getOptionalType(propType) :
Expand Down Expand Up @@ -13904,7 +13896,7 @@ namespace ts {
const checkType = <TypeParameter>root.checkType;
const instantiatedType = getMappedType(checkType, mapper);
if (checkType !== instantiatedType && instantiatedType.flags & (TypeFlags.Union | TypeFlags.Never)) {
return mapType(instantiatedType, t => getConditionalType(root, createReplacementMapper(checkType, t, mapper)));
return mapType(instantiatedType, t => getConditionalType(root, prependTypeMapping(checkType, t, mapper)));
}
}
return getConditionalType(root, mapper);
Expand Down Expand Up @@ -18743,7 +18735,7 @@ namespace ts {
if (defaultType) {
// Instantiate the default type. Any forward reference to a type
// parameter should be instantiated to the empty object type.
inferredType = instantiateType(defaultType, combineTypeMappers(createBackreferenceMapper(context, index), context.nonFixingMapper));
inferredType = instantiateType(defaultType, mergeTypeMappers(createBackreferenceMapper(context, index), context.nonFixingMapper));
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4880,14 +4880,15 @@ namespace ts {
Array,
Function,
Composite,
Merged,
}

/* @internal */
export type TypeMapper =
| { kind: TypeMapKind.Simple, source1: Type, target1: Type, source2: Type, target2: Type }
| { kind: TypeMapKind.Simple, source: Type, target: Type }
| { kind: TypeMapKind.Array, sources: readonly Type[], targets: readonly Type[] | undefined }
| { kind: TypeMapKind.Function, func: (t: Type) => Type }
| { kind: TypeMapKind.Composite, mapper1: TypeMapper, mapper2: TypeMapper };
| { kind: TypeMapKind.Composite | TypeMapKind.Merged, mapper1: TypeMapper, mapper2: TypeMapper, cachedSource: Type, cachedTarget: Type };

export const enum InferencePriority {
NakedTypeVariable = 1 << 0, // Naked type variable in union or intersection type
Expand Down
0