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
jakebailey 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 8 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
53 changes: 30 additions & 23 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -786,10 +786,10 @@ namespace ts {

const anyType = createIntrinsicType(TypeFlags.Any, "any");
const autoType = createIntrinsicType(TypeFlags.Any, "any");
autoType.objectFlags |= ObjectFlags.NonInferrableType;
Comment thread
jakebailey marked this conversation as resolved.
Outdated
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 +818,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 +9455,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 @@ -17196,6 +17191,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, /*excludeKinds*/ 0) : 0;

@weswigham weswigham Jul 13, 2022

Copy link
Copy Markdown
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"...)

@jakebailey jakebailey Jul 13, 2022

Copy link
Copy Markdown
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.

@jakebailey jakebailey Jul 13, 2022

Copy link
Copy Markdown
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).

return result;
}

Expand Down Expand Up @@ -22422,10 +22418,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 +22480,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 @@ -31263,7 +31270,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 +31905,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/issue43962.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
//// [issue43962.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)),
));


//// [issue43962.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); })); });
138 changes: 138 additions & 0 deletions tests/baselines/reference/issue43962.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
=== tests/cases/compiler/issue43962.ts ===
type Op<I, O> = (thing: Thing<I>) => Thing<O>;
>Op : Symbol(Op, Decl(issue43962.ts, 0, 0))
>I : Symbol(I, Decl(issue43962.ts, 0, 8))
>O : Symbol(O, Decl(issue43962.ts, 0, 10))
>thing : Symbol(thing, Decl(issue43962.ts, 0, 17))
>Thing : Symbol(Thing, Decl(issue43962.ts, 0, 46))
>I : Symbol(I, Decl(issue43962.ts, 0, 8))
>Thing : Symbol(Thing, Decl(issue43962.ts, 0, 46))
>O : Symbol(O, Decl(issue43962.ts, 0, 10))

type Thing<T> = {
>Thing : Symbol(Thing, Decl(issue43962.ts, 0, 46))
>T : Symbol(T, Decl(issue43962.ts, 1, 11))

value: T;
>value : Symbol(value, Decl(issue43962.ts, 1, 17))
>T : Symbol(T, Decl(issue43962.ts, 1, 11))

pipe<A, B>(
>pipe : Symbol(pipe, Decl(issue43962.ts, 2, 13))
>A : Symbol(A, Decl(issue43962.ts, 3, 9))
>B : Symbol(B, Decl(issue43962.ts, 3, 11))

opA: Op<T, A>,
>opA : Symbol(opA, Decl(issue43962.ts, 3, 15))
>Op : Symbol(Op, Decl(issue43962.ts, 0, 0))
>T : Symbol(T, Decl(issue43962.ts, 1, 11))
>A : Symbol(A, Decl(issue43962.ts, 3, 9))

opB: Op<A, B>,
>opB : Symbol(opB, Decl(issue43962.ts, 4, 22))
>Op : Symbol(Op, Decl(issue43962.ts, 0, 0))
>A : Symbol(A, Decl(issue43962.ts, 3, 9))
>B : Symbol(B, Decl(issue43962.ts, 3, 11))

): Thing<B>;
>Thing : Symbol(Thing, Decl(issue43962.ts, 0, 46))
>B : Symbol(B, Decl(issue43962.ts, 3, 11))

};
type Box<V> = { data: V };
>Box : Symbol(Box, Decl(issue43962.ts, 7, 2))
>V : Symbol(V, Decl(issue43962.ts, 8, 9))
>data : Symbol(data, Decl(issue43962.ts, 8, 15))
>V : Symbol(V, Decl(issue43962.ts, 8, 9))

declare const thing: Thing<number>;
>thing : Symbol(thing, Decl(issue43962.ts, 10, 13))
>Thing : Symbol(Thing, Decl(issue43962.ts, 0, 46))

declare function map<T, R>(project: (value: T) => R): Op<T, R>;
>map : Symbol(map, Decl(issue43962.ts, 10, 35))
>T : Symbol(T, Decl(issue43962.ts, 12, 21))
>R : Symbol(R, Decl(issue43962.ts, 12, 23))
>project : Symbol(project, Decl(issue43962.ts, 12, 27))
>value : Symbol(value, Decl(issue43962.ts, 12, 37))
>T : Symbol(T, Decl(issue43962.ts, 12, 21))
>R : Symbol(R, Decl(issue43962.ts, 12, 23))
>Op : Symbol(Op, Decl(issue43962.ts, 0, 0))
>T : Symbol(T, Decl(issue43962.ts, 12, 21))
>R : Symbol(R, Decl(issue43962.ts, 12, 23))

declare function tap<T>(next: (value: T) => void): Op<T, T>;
>tap : Symbol(tap, Decl(issue43962.ts, 12, 63))
>T : Symbol(T, Decl(issue43962.ts, 13, 21))
>next : Symbol(next, Decl(issue43962.ts, 13, 24))
>value : Symbol(value, Decl(issue43962.ts, 13, 31))
>T : Symbol(T, Decl(issue43962.ts, 13, 21))
>Op : Symbol(Op, Decl(issue43962.ts, 0, 0))
>T : Symbol(T, Decl(issue43962.ts, 13, 21))
>T : Symbol(T, Decl(issue43962.ts, 13, 21))

declare function box<V>(data: V): Box<V>;
>box : Symbol(box, Decl(issue43962.ts, 13, 60))
>V : Symbol(V, Decl(issue43962.ts, 14, 21))
>data : Symbol(data, Decl(issue43962.ts, 14, 24))
>V : Symbol(V, Decl(issue43962.ts, 14, 21))
>Box : Symbol(Box, Decl(issue43962.ts, 7, 2))
>V : Symbol(V, Decl(issue43962.ts, 14, 21))

declare function createAndUnbox<V>(factory: () => Thing<V | Box<V>>): Thing<V>;
>createAndUnbox : Symbol(createAndUnbox, Decl(issue43962.ts, 14, 41))
>V : Symbol(V, Decl(issue43962.ts, 15, 32))
>factory : Symbol(factory, Decl(issue43962.ts, 15, 35))
>Thing : Symbol(Thing, Decl(issue43962.ts, 0, 46))
>V : Symbol(V, Decl(issue43962.ts, 15, 32))
>Box : Symbol(Box, Decl(issue43962.ts, 7, 2))
>V : Symbol(V, Decl(issue43962.ts, 15, 32))
>Thing : Symbol(Thing, Decl(issue43962.ts, 0, 46))
>V : Symbol(V, Decl(issue43962.ts, 15, 32))

declare function log(value: any): void;
>log : Symbol(log, Decl(issue43962.ts, 15, 79))
>value : Symbol(value, Decl(issue43962.ts, 16, 21))

const result1 = createAndUnbox(() => thing.pipe(
>result1 : Symbol(result1, Decl(issue43962.ts, 18, 5))
>createAndUnbox : Symbol(createAndUnbox, Decl(issue43962.ts, 14, 41))
>thing.pipe : Symbol(pipe, Decl(issue43962.ts, 2, 13))
>thing : Symbol(thing, Decl(issue43962.ts, 10, 13))
>pipe : Symbol(pipe, Decl(issue43962.ts, 2, 13))

map((data) => box(data)),
>map : Symbol(map, Decl(issue43962.ts, 10, 35))
>data : Symbol(data, Decl(issue43962.ts, 19, 9))
>box : Symbol(box, Decl(issue43962.ts, 13, 60))
>data : Symbol(data, Decl(issue43962.ts, 19, 9))

tap((v) => log(v)),
>tap : Symbol(tap, Decl(issue43962.ts, 12, 63))
>v : Symbol(v, Decl(issue43962.ts, 20, 9))
>log : Symbol(log, Decl(issue43962.ts, 15, 79))
>v : Symbol(v, Decl(issue43962.ts, 20, 9))

));

const result2 = createAndUnbox(() => thing.pipe(
>result2 : Symbol(result2, Decl(issue43962.ts, 23, 5))
>createAndUnbox : Symbol(createAndUnbox, Decl(issue43962.ts, 14, 41))
>thing.pipe : Symbol(pipe, Decl(issue43962.ts, 2, 13))
>thing : Symbol(thing, Decl(issue43962.ts, 10, 13))
>pipe : Symbol(pipe, Decl(issue43962.ts, 2, 13))

tap((v) => log(v)),
>tap : Symbol(tap, Decl(issue43962.ts, 12, 63))
>v : Symbol(v, Decl(issue43962.ts, 24, 9))
>log : Symbol(log, Decl(issue43962.ts, 15, 79))
>v : Symbol(v, Decl(issue43962.ts, 24, 9))

map((data) => box(data)),
>map : Symbol(map, Decl(issue43962.ts, 10, 35))
>data : Symbol(data, Decl(issue43962.ts, 25, 9))
>box : Symbol(box, Decl(issue43962.ts, 13, 60))
>data : Symbol(data, Decl(issue43962.ts, 25, 9))

));

Loading