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.

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

Reduce intersections by discriminants #36696

Merged
merged 35 commits into from
Feb 29, 2020
Merged

Conversation

ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented Feb 8, 2020

We have long speculated that intersections of object types with discriminant properties of disjoint types should be eqivalent to never (the empty type), since objects of such types are not possible to construct. This PR finally implements that feature.

In addition to the intersection reduction implemented in #31838, an intersection T1 & T2 & ... & Tn is equivalent to never when

  • two or more of the Tx types have properties with the same name, and
  • in at least one group of properties with the same name, some property has a literal type and no property has type never, and
  • the intersection of the types of the properties in such a group is never.

An example:

type A = { kind: 'a', foo: string };
type B = { kind: 'b', foo: number };
type C = { kind: 'c', foo: number };

type AB = A & B;  // never
type BC = B & C;  // never

Previously, AB and BC appeared to be object-like types with kind properties of type never. Now, the types themselves are never because of the disjoint discriminant properties. This means that accessing properties on an empty intersection now produces errors, similarly to accessing properties on a never value:

declare let bc: B & C;
let foo = bc.foo;  // Error, but previously was ok

Because empty intersection types are equivalent to never, they disappear from union types:

declare let x: A | (B & C);
let a: A = x;  // Ok, but previousy was error

Previously, an error was reported in the example because x was considered to have a foo property of type string | number. With this PR there is no error because we treat B & C as never, and therefore A | (B & C) is equivalent to A.

It is obviously uncommon to have type annotations that explicitly intersect disjoint types, as in the examples above. The root cause of empty intersections is typically instantiations of generic types that intersect type parameters.

Note that elimination of empty intersections causes an observable difference between keyof (A & B) and keyof A | keyof B, where previously the two were always equivalent. The former now produces string | number | symbol (equivalent to keyof never), whereas the latter produces 'kind' | 'foo'.

This PR is technically a breaking change because code may exist that depends on empty intersections not being reduced away, even though this is incorrect from a typing perspective.

A note on the implementation of the feature:

In #31838 we reduce empty intersections to never immediately upon construction. However, in this PR we have to defer the determination because it requires us to resolve the types of properties in the constituent object types. Specifically, when two or more constituent object types have properties with the same name, we need to resolve the types of those properties in order to determine if they're disjoint discriminants, but doing so during intersection construction can easily cause circularities. So, instead of reducing intersections upon construction, we reduce them immediately before accessing members, relating them to other types, or converting them to their string representations in diagnostics and quick info. This is the purpose of the getReducedType function introduced by this PR.

Fixes #36736.

@ahejlsberg
Copy link
Member Author

@typescript-bot test this
@typescript-bot user test this
@typescript-bot run dt
@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 8, 2020

Heya @ahejlsberg, I've started to run the perf test suite on this PR at 94353d2. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 8, 2020

Heya @ahejlsberg, I've started to run the parallelized community code test suite on this PR at 94353d2. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 8, 2020

Heya @ahejlsberg, I've started to run the extended test suite on this PR at 94353d2. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 8, 2020

Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 94353d2. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot
Copy link
Collaborator

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

@ahejlsberg
Copy link
Member Author

@typescript-bot test this
@typescript-bot user test this
@typescript-bot run dt
@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 8, 2020

Heya @ahejlsberg, I've started to run the perf test suite on this PR at 346ec30. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 8, 2020

Heya @ahejlsberg, I've started to run the parallelized community code test suite on this PR at 346ec30. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 8, 2020

Heya @ahejlsberg, I've started to run the extended test suite on this PR at 346ec30. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 8, 2020

Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 346ec30. You can monitor the build here. It should now contribute to this PR's status checks.

@ahejlsberg
Copy link
Member Author

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 9, 2020

Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at acd3fec. You can monitor the build here. It should now contribute to this PR's status checks.

@ahejlsberg
Copy link
Member Author

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 10, 2020

Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 3a1ba48. You can monitor the build here. It should now contribute to this PR's status checks.

@ahejlsberg
Copy link
Member Author

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 10, 2020

Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 37c6280. You can monitor the build here. It should now contribute to this PR's status checks.

@ahejlsberg
Copy link
Member Author

@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 10, 2020

Heya @ahejlsberg, I've started to run the perf test suite on this PR at 37c6280. You can monitor the build here. It should now contribute to this PR's status checks.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

@ahejlsberg
The results of the perf run you requested are in!

Here they are:

Comparison Report - master..36696

Metric master 36696 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 356,865k (± 0.03%) 358,875k (± 0.03%) +2,010k (+ 0.56%) 358,634k 359,086k
Parse Time 1.61s (± 0.64%) 1.61s (± 0.43%) -0.00s (- 0.06%) 1.60s 1.63s
Bind Time 0.89s (± 0.50%) 0.88s (± 1.00%) -0.01s (- 0.90%) 0.86s 0.90s
Check Time 4.69s (± 0.53%) 4.69s (± 0.43%) -0.00s (- 0.09%) 4.66s 4.75s
Emit Time 5.23s (± 0.80%) 5.22s (± 0.51%) -0.01s (- 0.17%) 5.18s 5.29s
Total Time 12.42s (± 0.47%) 12.39s (± 0.36%) -0.03s (- 0.20%) 12.34s 12.54s
Monaco - node (v10.16.3, x64)
Memory used 364,650k (± 0.02%) 364,687k (± 0.01%) +37k (+ 0.01%) 364,615k 364,759k
Parse Time 1.25s (± 0.58%) 1.25s (± 0.80%) -0.00s (- 0.16%) 1.23s 1.27s
Bind Time 0.78s (± 0.75%) 0.77s (± 0.47%) -0.00s (- 0.39%) 0.77s 0.78s
Check Time 4.69s (± 0.42%) 4.68s (± 0.43%) -0.01s (- 0.21%) 4.64s 4.72s
Emit Time 2.90s (± 0.81%) 2.89s (± 0.89%) -0.01s (- 0.28%) 2.82s 2.93s
Total Time 9.62s (± 0.36%) 9.60s (± 0.46%) -0.02s (- 0.24%) 9.51s 9.67s
TFS - node (v10.16.3, x64)
Memory used 324,088k (± 0.01%) 324,207k (± 0.03%) +119k (+ 0.04%) 324,061k 324,410k
Parse Time 0.95s (± 0.52%) 0.95s (± 0.55%) -0.00s (- 0.31%) 0.94s 0.96s
Bind Time 0.75s (± 1.26%) 0.76s (± 1.88%) +0.01s (+ 1.07%) 0.71s 0.78s
Check Time 4.25s (± 0.58%) 4.28s (± 0.58%) +0.03s (+ 0.68%) 4.22s 4.35s
Emit Time 3.03s (± 0.86%) 3.01s (± 1.03%) -0.02s (- 0.59%) 2.93s 3.07s
Total Time 8.98s (± 0.60%) 8.99s (± 0.54%) +0.01s (+ 0.14%) 8.90s 9.14s
Angular - node (v12.1.0, x64)
Memory used 332,704k (± 0.03%) 334,652k (± 0.03%) +1,948k (+ 0.59%) 334,483k 334,823k
Parse Time 1.57s (± 0.60%) 1.57s (± 0.57%) -0.00s (- 0.06%) 1.55s 1.59s
Bind Time 0.87s (± 0.75%) 0.87s (± 0.56%) +0.01s (+ 0.69%) 0.86s 0.88s
Check Time 4.58s (± 0.61%) 4.60s (± 0.40%) +0.02s (+ 0.48%) 4.56s 4.64s
Emit Time 5.41s (± 0.59%) 5.41s (± 0.69%) -0.01s (- 0.11%) 5.29s 5.47s
Total Time 12.43s (± 0.43%) 12.45s (± 0.36%) +0.02s (+ 0.18%) 12.36s 12.55s
Monaco - node (v12.1.0, x64)
Memory used 344,537k (± 0.02%) 344,588k (± 0.01%) +51k (+ 0.01%) 344,479k 344,716k
Parse Time 1.21s (± 0.87%) 1.22s (± 0.74%) +0.00s (+ 0.08%) 1.20s 1.23s
Bind Time 0.75s (± 0.78%) 0.75s (± 0.99%) +0.01s (+ 0.80%) 0.74s 0.77s
Check Time 4.54s (± 0.44%) 4.55s (± 0.52%) +0.01s (+ 0.26%) 4.50s 4.60s
Emit Time 2.96s (± 0.61%) 2.95s (± 0.89%) -0.00s (- 0.14%) 2.90s 3.01s
Total Time 9.45s (± 0.40%) 9.47s (± 0.44%) +0.02s (+ 0.21%) 9.39s 9.56s
TFS - node (v12.1.0, x64)
Memory used 306,435k (± 0.02%) 306,452k (± 0.01%) +16k (+ 0.01%) 306,372k 306,571k
Parse Time 0.94s (± 0.80%) 0.94s (± 0.85%) -0.00s (- 0.21%) 0.93s 0.97s
Bind Time 0.71s (± 0.94%) 0.71s (± 1.07%) -0.00s (- 0.56%) 0.69s 0.73s
Check Time 4.17s (± 0.46%) 4.17s (± 0.49%) +0.00s (+ 0.05%) 4.13s 4.21s
Emit Time 3.08s (± 0.48%) 3.09s (± 0.88%) +0.01s (+ 0.29%) 3.03s 3.16s
Total Time 8.91s (± 0.21%) 8.91s (± 0.46%) +0.00s (+ 0.03%) 8.83s 9.02s
Angular - node (v8.9.0, x64)
Memory used 351,858k (± 0.01%) 353,918k (± 0.02%) +2,060k (+ 0.59%) 353,821k 354,052k
Parse Time 2.09s (± 0.42%) 2.12s (± 0.45%) +0.02s (+ 1.10%) 2.10s 2.15s
Bind Time 0.93s (± 1.09%) 0.94s (± 0.73%) +0.01s (+ 1.08%) 0.92s 0.95s
Check Time 5.45s (± 0.42%) 5.49s (± 0.66%) +0.03s (+ 0.61%) 5.39s 5.55s
Emit Time 6.20s (± 0.45%) 6.29s (± 1.09%) +0.09s (+ 1.52%) 6.16s 6.46s
Total Time 14.67s (± 0.33%) 14.83s (± 0.67%) +0.16s (+ 1.08%) 14.61s 15.00s
Monaco - node (v8.9.0, x64)
Memory used 362,969k (± 0.01%) 362,972k (± 0.01%) +4k (+ 0.00%) 362,859k 363,064k
Parse Time 1.56s (± 0.43%) 1.56s (± 0.37%) +0.00s (+ 0.19%) 1.55s 1.58s
Bind Time 0.95s (± 0.61%) 0.95s (± 0.68%) +0.00s (+ 0.11%) 0.94s 0.96s
Check Time 5.43s (± 1.60%) 5.44s (± 1.49%) +0.01s (+ 0.15%) 5.29s 5.56s
Emit Time 3.26s (± 4.78%) 3.17s (± 3.29%) -0.09s (- 2.79%) 2.98s 3.34s
Total Time 11.19s (± 0.69%) 11.12s (± 0.23%) -0.07s (- 0.67%) 11.07s 11.18s
TFS - node (v8.9.0, x64)
Memory used 323,469k (± 0.01%) 323,494k (± 0.01%) +26k (+ 0.01%) 323,387k 323,625k
Parse Time 1.26s (± 0.24%) 1.26s (± 0.44%) 0.00s ( 0.00%) 1.25s 1.27s
Bind Time 0.76s (± 0.63%) 0.76s (± 0.48%) -0.00s (- 0.13%) 0.75s 0.76s
Check Time 4.84s (± 0.51%) 4.83s (± 0.26%) -0.01s (- 0.27%) 4.79s 4.85s
Emit Time 3.20s (± 0.77%) 3.19s (± 0.77%) -0.01s (- 0.22%) 3.14s 3.27s
Total Time 10.06s (± 0.38%) 10.04s (± 0.35%) -0.02s (- 0.21%) 9.95s 10.12s
Angular - node (v8.9.0, x86)
Memory used 200,066k (± 0.04%) 201,176k (± 0.03%) +1,110k (+ 0.55%) 201,031k 201,317k
Parse Time 2.04s (± 0.98%) 2.06s (± 0.79%) +0.02s (+ 1.18%) 2.03s 2.11s
Bind Time 1.05s (± 0.90%) 1.05s (± 0.55%) +0.00s (+ 0.29%) 1.04s 1.06s
Check Time 4.97s (± 0.59%) 4.99s (± 0.47%) +0.02s (+ 0.38%) 4.95s 5.04s
Emit Time 6.13s (± 2.06%) 6.17s (± 1.74%) +0.04s (+ 0.67%) 6.00s 6.49s
Total Time 14.18s (± 1.01%) 14.27s (± 0.86%) +0.09s (+ 0.61%) 14.06s 14.63s
Monaco - node (v8.9.0, x86)
Memory used 203,771k (± 0.02%) 203,815k (± 0.02%) +45k (+ 0.02%) 203,688k 203,855k
Parse Time 1.60s (± 0.67%) 1.60s (± 0.30%) -0.00s (- 0.25%) 1.59s 1.61s
Bind Time 0.77s (± 1.09%) 0.77s (± 1.12%) +0.00s (+ 0.13%) 0.76s 0.80s
Check Time 5.17s (± 1.67%) 5.15s (± 1.00%) -0.02s (- 0.39%) 5.07s 5.26s
Emit Time 3.12s (± 2.63%) 3.16s (± 1.67%) +0.04s (+ 1.28%) 3.01s 3.23s
Total Time 10.67s (± 0.36%) 10.68s (± 0.34%) +0.02s (+ 0.15%) 10.61s 10.73s
TFS - node (v8.9.0, x86)
Memory used 182,630k (± 0.02%) 182,679k (± 0.02%) +49k (+ 0.03%) 182,613k 182,804k
Parse Time 1.30s (± 0.98%) 1.30s (± 0.68%) +0.00s (+ 0.08%) 1.28s 1.32s
Bind Time 0.71s (± 1.51%) 0.71s (± 0.81%) -0.00s (- 0.14%) 0.70s 0.73s
Check Time 4.60s (± 0.56%) 4.59s (± 0.55%) -0.01s (- 0.24%) 4.55s 4.68s
Emit Time 2.95s (± 1.07%) 2.96s (± 1.00%) +0.01s (+ 0.37%) 2.91s 3.05s
Total Time 9.57s (± 0.53%) 9.57s (± 0.32%) -0.00s (- 0.02%) 9.50s 9.65s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-166-generic
Architecturex64
Available Memory16 GB
Available Memory2 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v10.16.3, x64)
  • node (v12.1.0, x64)
  • node (v8.9.0, x64)
  • node (v8.9.0, x86)
Scenarios
  • Angular - node (v10.16.3, x64)
  • Angular - node (v12.1.0, x64)
  • Angular - node (v8.9.0, x64)
  • Angular - node (v8.9.0, x86)
  • Monaco - node (v10.16.3, x64)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v8.9.0, x64)
  • Monaco - node (v8.9.0, x86)
  • TFS - node (v10.16.3, x64)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v8.9.0, x64)
  • TFS - node (v8.9.0, x86)
Benchmark Name Iterations
Current 36696 10
Baseline master 10

@andrewvarga
Copy link

Since updating to 3.9 I have a compile error which I suspect is caused by this PR. The error might be valid, but could someone explain to me why?
Here is a sample code:

class A {
    private _name = "A";
    public y: number = 1;
}

class B {
    private _name = "B"
    public y: number = 2;
}

class C {
    public y: number = 3;
}

let c: A & B = new C();
// Error: Type 'C' is not assignable to type 'never'.
// The intersection 'A & B' was reduced to 'never' because property '_name' 
// exists in multiple constituents and is private in some.(2322)

What I'm not getting is why the private variable would have any say in this? I thought doing something like A & B should only look at the public interfaces of these objects, anything private wouldn't have any effect on the resulting type.

GordonSmith added a commit to GordonSmith/lumino that referenced this pull request May 21, 2020
Generate type files for older typescript compilers

Fixed issues relating to:  microsoft/TypeScript#36696

Signed-off-by: Gordon Smith <GordonJSmith@gmail.com>
GordonSmith added a commit to GordonSmith/lumino that referenced this pull request May 21, 2020
Generate type files for older typescript compilers

Fixed issues relating to:  microsoft/TypeScript#36696

Signed-off-by: Gordon Smith <GordonJSmith@gmail.com>
GordonSmith added a commit to GordonSmith/lumino that referenced this pull request May 22, 2020
Generate type files for older typescript compilers

Fixed issues relating to:  microsoft/TypeScript#36696

Signed-off-by: Gordon Smith <GordonJSmith@gmail.com>
GordonSmith added a commit to GordonSmith/lumino that referenced this pull request May 22, 2020
Generate type files for older typescript compilers

Fixed issues relating to:  microsoft/TypeScript#36696

Signed-off-by: Gordon Smith <GordonJSmith@gmail.com>
GordonSmith added a commit to GordonSmith/lumino that referenced this pull request May 25, 2020
Generate type files for older typescript compilers

Fixed issues relating to:  microsoft/TypeScript#36696

Signed-off-by: Gordon Smith <GordonJSmith@gmail.com>
@weswigham
Copy link
Member

weswigham commented Oct 22, 2020

@andrewvarga This is a bit of a late comment, but it's because private class members, while their types aren't externally visible, nominally tag the containing class. These nominal tags are mutually exclusive, and are what result in the reduction (the field cannot possibly be satisfied by anything other than that class itself). Hence the error: The intersection 'A & B' was reduced to 'never' because property '_name' exists in multiple constituents and is private in some.

lowr added a commit to lowr/TypeScript that referenced this pull request Feb 6, 2022
When types are checked if they are `never` by their `TypeFlags`, they
must first be reduced, because intersection types of object-like types
which reduce to `never` don't have `TypeFlags.Never`. See microsoft#36696 for
details.

When a function is declared to return such type and it doesn't contain
any return statement or it only contains return statements without
expression, the error messages are incorrect as the checker sees its
return type as non-`never`.

This commit takes into account that intersection types potentially
reduces to `never` and improves error messages.
lowr added a commit to lowr/TypeScript that referenced this pull request Feb 6, 2022
When types are checked if they are `never` by their `TypeFlags`, they
must first be reduced, because intersection types of object-like types
which reduce to `never` don't have `TypeFlags.Never`. See microsoft#36696 for
details.

When a function is declared to return such type and it doesn't contain
any return statement or it only contains return statements without
expression, the error messages are incorrect as the checker sees its
return type as non-`never`.

This commit takes into account that intersection types potentially
reduces to `never` and improves error messages.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Would introduce errors in existing code
5 participants