Skip to content

Properly propagate ObjectFlags.NonInferrableType, clean up non-inferrable code paths #49887

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 13 commits into from
Jul 15, 2022
Merged

Properly propagate ObjectFlags.NonInferrableType, clean up non-inferrable code paths #49887

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
f6a2bed
Add test
jakebailey Jul 7, 2022
f5db542
Ensure flags are propogated when instantiating anonymous types
jakebailey Jul 12, 2022
3f02cba
Always return on equal aliasSymbol
jakebailey Jul 13, 2022
b875b3e
Just use flag to check for non-inferrable types
jakebailey Jul 13, 2022
66dbd1d
Eliminate nonInferrableType
jakebailey Jul 13, 2022
0db958c
Also remove nonInferrableAnyType, which looks like it should have fla…
jakebailey Jul 13, 2022
6dad0a4
Make autoType non-inferrable instead of checking explicitly
jakebailey Jul 13, 2022
2974d9a
Rewrite and move comment to the correct place
jakebailey Jul 13, 2022
dd6bdaf
Give autoType its flags the normal way
jakebailey Jul 13, 2022
4a73c1c
Make excludesKind optional in getPropagatingFlagsOfTypes
jakebailey Jul 13, 2022
0972ebd
Rename test
jakebailey Jul 13, 2022
28bc9d6
Add test from fp-ts
jakebailey Jul 13, 2022
9e831ec
Remove old hack that skipped parameters during inference; this was re…
jakebailey Jul 14, 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
81 changes: 42 additions & 39 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -785,11 +785,10 @@ namespace ts {
const errorTypes = new Map<string, Type>();

const anyType = createIntrinsicType(TypeFlags.Any, "any");
const autoType = createIntrinsicType(TypeFlags.Any, "any");
const autoType = createIntrinsicType(TypeFlags.Any, "any", ObjectFlags.NonInferrableType);
const wildcardType = createIntrinsicType(TypeFlags.Any, "any");
const errorType = createIntrinsicType(TypeFlags.Any, "error");
const unresolvedType = createIntrinsicType(TypeFlags.Any, "unresolved");
const nonInferrableAnyType = createIntrinsicType(TypeFlags.Any, "any", ObjectFlags.ContainsWideningType);
const intrinsicMarkerType = createIntrinsicType(TypeFlags.Any, "intrinsic");
const unknownType = createIntrinsicType(TypeFlags.Unknown, "unknown");
const nonNullUnknownType = createIntrinsicType(TypeFlags.Unknown, "unknown");
Expand Down Expand Up @@ -818,8 +817,7 @@ namespace ts {
const esSymbolType = createIntrinsicType(TypeFlags.ESSymbol, "symbol");
const voidType = createIntrinsicType(TypeFlags.Void, "void");
const neverType = createIntrinsicType(TypeFlags.Never, "never");
const silentNeverType = createIntrinsicType(TypeFlags.Never, "never");
const nonInferrableType = createIntrinsicType(TypeFlags.Never, "never", ObjectFlags.NonInferrableType);
const silentNeverType = createIntrinsicType(TypeFlags.Never, "never", ObjectFlags.NonInferrableType);
const implicitNeverType = createIntrinsicType(TypeFlags.Never, "never");
const unreachableNeverType = createIntrinsicType(TypeFlags.Never, "never");
const nonPrimitiveType = createIntrinsicType(TypeFlags.NonPrimitive, "object");
Expand Down Expand Up @@ -9456,11 +9454,7 @@ namespace ts {
if (reportErrors && !declarationBelongsToPrivateAmbientMember(element)) {
reportImplicitAny(element, anyType);
}
// When we're including the pattern in the type (an indication we're obtaining a contextual type), we
// use the non-inferrable any type. Inference will never directly infer this type, but it is possible
// to infer a type that contains it, e.g. for a binding pattern like [foo] or { foo }. In such cases,
// widening of the binding pattern type substitutes a regular any for the non-inferrable any.
return includePatternInType ? nonInferrableAnyType : anyType;
return anyType;
}

// Return the type implied by an object binding pattern
Expand Down Expand Up @@ -13499,12 +13493,12 @@ namespace ts {

// This function is used to propagate certain flags when creating new object type references and union types.
// It is only necessary to do so if a constituent type might be the undefined type, the null type, the type
// of an object literal or the anyFunctionType. This is because there are operations in the type checker
// of an object literal or a non-inferrable type. This is because there are operations in the type checker
// that care about the presence of such types at arbitrary depth in a containing type.
function getPropagatingFlagsOfTypes(types: readonly Type[], excludeKinds: TypeFlags): ObjectFlags {
function getPropagatingFlagsOfTypes(types: readonly Type[], excludeKinds?: TypeFlags): ObjectFlags {
let result: ObjectFlags = 0;
for (const type of types) {
if (!(type.flags & excludeKinds)) {
if (excludeKinds === undefined || !(type.flags & excludeKinds)) {
result |= getObjectFlags(type);
}
}
Expand All @@ -13517,7 +13511,7 @@ namespace ts {
if (!type) {
type = createObjectType(ObjectFlags.Reference, target.symbol) as TypeReference;
target.instantiations.set(id, type);
type.objectFlags |= typeArguments ? getPropagatingFlagsOfTypes(typeArguments, /*excludeKinds*/ 0) : 0;
type.objectFlags |= typeArguments ? getPropagatingFlagsOfTypes(typeArguments) : 0;
type.target = target;
type.resolvedTypeArguments = typeArguments;
}
Expand Down Expand Up @@ -17196,6 +17190,7 @@ namespace ts {
result.mapper = mapper;
result.aliasSymbol = aliasSymbol || type.aliasSymbol;
result.aliasTypeArguments = aliasSymbol ? aliasTypeArguments : instantiateTypes(type.aliasTypeArguments, mapper);
result.objectFlags |= result.aliasTypeArguments ? getPropagatingFlagsOfTypes(result.aliasTypeArguments) : 0;
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.

Huh, this is interesting because this is redundant for TypeReference anonymous types (since those internally collect propagating flags from their type arguments), but required for any other anonymous object type with an alias. Go figure. I wonder if we should be propagating these object flags through conditional types, too... We already preserve them through intersections and unions. (We probably should be, since conditionals are "smart unions"...)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I think they should. Right now I'm trying to go by hand to every createAnonymousType, createObjectType,... etc and see if there's anything missed, and it's a bit daunting because there's like 100 of them to verify, and nothing I've changed so far as actually changed any test.

I want to have something I can run while I'm debugging that'll observe each type during inference and walk down to see if there was a NonInferrableType flag that was missed, but I get stuck in stack overflows, so I don't quite know how I can do my sanity check.

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 think for this PR, I'm inclined to not try and go everywhere and add some more flag propagation without a way for me to check these; I just don't trust that I'm getting it right, and I don't have any way to observe the result.

This PR at least improves the one case I can test, and has a good amount of cleanup, so I'm happy with it (but I'll rerun the testing since I changed the structure of this quite a bit).

}

Expand Down Expand Up @@ -22422,10 +22417,13 @@ namespace ts {
propagationType = savePropagationType;
return;
}
if (source.aliasSymbol && source.aliasTypeArguments && source.aliasSymbol === target.aliasSymbol) {
// Source and target are types originating in the same generic type alias declaration.
// Simply infer from source type arguments to target type arguments.
inferFromTypeArguments(source.aliasTypeArguments, target.aliasTypeArguments!, getAliasVariances(source.aliasSymbol));
if (source.aliasSymbol && source.aliasSymbol === target.aliasSymbol) {
if (source.aliasTypeArguments) {
// Source and target are types originating in the same generic type alias declaration.
// Simply infer from source type arguments to target type arguments.
inferFromTypeArguments(source.aliasTypeArguments, target.aliasTypeArguments!, getAliasVariances(source.aliasSymbol));
}
// And if there weren't any type arguments, there's no reason to run inference as the types must be the same.
return;
}
if (source === target && source.flags & TypeFlags.UnionOrIntersection) {
Expand Down Expand Up @@ -22481,18 +22479,26 @@ namespace ts {
target = getActualTypeVariable(target);
}
if (target.flags & TypeFlags.TypeVariable) {
// If target is a type parameter, make an inference, unless the source type contains
// the anyFunctionType (the wildcard type that's used to avoid contextually typing functions).
// Because the anyFunctionType is internal, it should not be exposed to the user by adding
// it as an inference candidate. Hopefully, a better candidate will come along that does
// not contain anyFunctionType when we come back to this argument for its second round
// of inference. Also, we exclude inferences for silentNeverType (which is used as a wildcard
// when constructing types from type parameters that had no inference candidates).
if (source === nonInferrableAnyType || source === silentNeverType || (priority & InferencePriority.ReturnType && (source === autoType || source === autoArrayType)) || isFromInferenceBlockedSource(source)) {
// Skip inference if the source is "blocked", which is used by the language service to
// prevent inference on nodes currently being edited.
if (isFromInferenceBlockedSource(source)) {
return;
}
const inference = getInferenceInfoForType(target);
if (inference) {
// If target is a type parameter, make an inference, unless the source type contains
// a "non-inferrable" type. Types with this flag set are markers used to prevent inference.
//
// For example:
// - anyFunctionType is a wildcard type that's used to avoid contextually typing functions;
// it's internal, so should not be exposed to the user by adding it as a candidate.
// - autoType (and autoArrayType) is a special "any" used in control flow; like anyFunctionType,
// it's internal and should not be observable.
// - silentNeverType is returned by getInferredType when instantiating a generic function for
// inference (and a type variable has no mapping).
//
// This flag is infectious; if we produce Box<never> (where never is silentNeverType), Box<never> is
// also non-inferrable.
if (getObjectFlags(source) & ObjectFlags.NonInferrableType) {
return;
}
Expand Down Expand Up @@ -23043,21 +23049,18 @@ namespace ts {
const sourceLen = sourceSignatures.length;
const targetLen = targetSignatures.length;
const len = sourceLen < targetLen ? sourceLen : targetLen;
const skipParameters = !!(getObjectFlags(source) & ObjectFlags.NonInferrableType);
for (let i = 0; i < len; i++) {
inferFromSignature(getBaseSignature(sourceSignatures[sourceLen - len + i]), getErasedSignature(targetSignatures[targetLen - len + i]), skipParameters);
inferFromSignature(getBaseSignature(sourceSignatures[sourceLen - len + i]), getErasedSignature(targetSignatures[targetLen - len + i]));
}
}

function inferFromSignature(source: Signature, target: Signature, skipParameters: boolean) {
if (!skipParameters) {
const saveBivariant = bivariant;
const kind = target.declaration ? target.declaration.kind : SyntaxKind.Unknown;
// Once we descend into a bivariant signature we remain bivariant for all nested inferences
bivariant = bivariant || kind === SyntaxKind.MethodDeclaration || kind === SyntaxKind.MethodSignature || kind === SyntaxKind.Constructor;
applyToParameterTypes(source, target, inferFromContravariantTypes);
bivariant = saveBivariant;
}
function inferFromSignature(source: Signature, target: Signature) {
const saveBivariant = bivariant;
const kind = target.declaration ? target.declaration.kind : SyntaxKind.Unknown;
// Once we descend into a bivariant signature we remain bivariant for all nested inferences
bivariant = bivariant || kind === SyntaxKind.MethodDeclaration || kind === SyntaxKind.MethodSignature || kind === SyntaxKind.Constructor;
applyToParameterTypes(source, target, inferFromContravariantTypes);
bivariant = saveBivariant;
applyToReturnTypes(source, target, inferFromTypes);
}

Expand Down Expand Up @@ -31263,7 +31266,7 @@ namespace ts {
// returns a function type, we choose to defer processing. This narrowly permits function composition
// operators to flow inferences through return types, but otherwise processes calls right away. We
// use the resolvingSignature singleton to indicate that we deferred processing. This result will be
// propagated out and eventually turned into nonInferrableType (a type that is assignable to anything and
// propagated out and eventually turned into silentNeverType (a type that is assignable to anything and
// from which we never make inferences).
if (checkMode & CheckMode.SkipGenericFunctions && !node.typeArguments && callSignatures.some(isGenericFunctionReturningFunction)) {
skippedGenericFunction(node, checkMode);
Expand Down Expand Up @@ -31898,8 +31901,8 @@ namespace ts {
const signature = getResolvedSignature(node, /*candidatesOutArray*/ undefined, checkMode);
if (signature === resolvingSignature) {
// CheckMode.SkipGenericFunctions is enabled and this is a call to a generic function that
// returns a function type. We defer checking and return nonInferrableType.
return nonInferrableType;
// returns a function type. We defer checking and return silentNeverType.
return silentNeverType;
}

checkDeprecatedSignature(signature, node);
Expand Down
34 changes: 34 additions & 0 deletions tests/baselines/reference/nonInferrableTypePropagation1.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
//// [nonInferrableTypePropagation1.ts]
type Op<I, O> = (thing: Thing<I>) => Thing<O>;
type Thing<T> = {
value: T;
pipe<A, B>(
opA: Op<T, A>,
opB: Op<A, B>,
): Thing<B>;
};
type Box<V> = { data: V };

declare const thing: Thing<number>;

declare function map<T, R>(project: (value: T) => R): Op<T, R>;
declare function tap<T>(next: (value: T) => void): Op<T, T>;
declare function box<V>(data: V): Box<V>;
declare function createAndUnbox<V>(factory: () => Thing<V | Box<V>>): Thing<V>;
declare function log(value: any): void;

const result1 = createAndUnbox(() => thing.pipe(
map((data) => box(data)),
tap((v) => log(v)),
));

const result2 = createAndUnbox(() => thing.pipe(
tap((v) => log(v)),
map((data) => box(data)),
));


//// [nonInferrableTypePropagation1.js]
"use strict";
var result1 = createAndUnbox(function () { return thing.pipe(map(function (data) { return box(data); }), tap(function (v) { return log(v); })); });
var result2 = createAndUnbox(function () { return thing.pipe(tap(function (v) { return log(v); }), map(function (data) { return box(data); })); });
Loading