Skip to content

Fix33448 - #35513

Merged
Anders Hejlsberg (ahejlsberg) merged 5 commits into
masterfrom
fix33448
Dec 12, 2019
Merged

Fix33448#35513
Anders Hejlsberg (ahejlsberg) merged 5 commits into
masterfrom
fix33448

Conversation

@ahejlsberg

Copy link
Copy Markdown
Member

Fixes#33448. Also fixes the issue in #33498 (comment).

This PR supercedes #33498. (Thanks Jack Williams (@jack-williams)!)

@ahejlsberg

Copy link
Copy Markdown
MemberAuthor

@typescript-bot

TypeScript Bot (typescript-bot) commented Dec 5, 2019

Copy link
Copy Markdown
Contributor

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

@typescript-bot

TypeScript Bot (typescript-bot) commented Dec 5, 2019

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the extended test suite on this PR at 6bc8b12. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot

TypeScript Bot (typescript-bot) commented Dec 5, 2019

Copy link
Copy Markdown
Contributor

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

@typescript-bot

TypeScript Bot (typescript-bot) commented Dec 5, 2019

Copy link
Copy Markdown
Contributor

Heya Anders Hejlsberg (@ahejlsberg), I've started to run the perf test suite on this PR at 6bc8b12. 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
Copy Markdown
Contributor

Anders Hejlsberg (@ahejlsberg)
The results of the perf run you requested are in!

Here they are:

Comparison Report - master..35513

Metricmaster35513DeltaBestWorst
Angular - node (v10.16.3, x64)
Memory used354,807k (± 0.02%)356,542k (± 0.02%)+1,736k (+ 0.49%)356,315k356,620k
Parse Time1.61s (± 0.60%)1.63s (± 0.35%)+0.02s (+ 1.31%)1.62s1.64s
Bind Time0.86s (± 0.95%)0.86s (± 0.77%)+0.00s (+ 0.35%)0.85s0.87s
Check Time4.53s (± 0.27%)4.53s (± 0.61%)+0.01s (+ 0.13%)4.49s4.60s
Emit Time5.25s (± 0.47%)5.22s (± 0.71%)-0.02s (- 0.38%)5.16s5.31s
Total Time12.24s (± 0.28%)12.25s (± 0.43%)+0.01s (+ 0.09%)12.17s12.38s
Monaco - node (v10.16.3, x64)
Memory used366,213k (± 0.01%)366,145k (± 0.01%)-68k (- 0.02%)366,038k366,273k
Parse Time1.25s (± 0.61%)1.26s (± 0.85%)+0.01s (+ 0.48%)1.25s1.29s
Bind Time0.76s (± 0.49%)0.76s (± 0.48%)+0.00s (+ 0.13%)0.75s0.76s
Check Time4.65s (± 0.59%)4.66s (± 0.64%)+0.01s (+ 0.28%)4.58s4.72s
Emit Time2.93s (± 0.71%)2.94s (± 0.50%)+0.01s (+ 0.20%)2.91s2.98s
Total Time9.58s (± 0.40%)9.61s (± 0.48%)+0.03s (+ 0.28%)9.51s9.73s
TFS - node (v10.16.3, x64)
Memory used321,970k (± 0.03%)321,977k (± 0.01%)+7k (+ 0.00%)321,915k322,039k
Parse Time0.95s (± 0.71%)0.95s (± 0.86%)-0.00s (- 0.10%)0.94s0.97s
Bind Time0.73s (± 1.12%)0.72s (± 1.38%)-0.01s (- 0.96%)0.70s0.74s
Check Time4.11s (± 0.35%)4.14s (± 0.35%)+0.03s (+ 0.66%)4.10s4.17s
Emit Time3.04s (± 0.78%)3.06s (± 0.65%)+0.01s (+ 0.39%)3.00s3.10s
Total Time8.83s (± 0.34%)8.87s (± 0.33%)+0.03s (+ 0.38%)8.82s8.95s
Angular - node (v12.1.0, x64)
Memory used330,399k (± 0.04%)332,012k (± 0.01%)+1,612k (+ 0.49%)331,945k332,113k
Parse Time1.57s (± 0.63%)1.59s (± 0.56%)+0.02s (+ 1.08%)1.57s1.61s
Bind Time0.84s (± 0.68%)0.85s (± 0.52%)+0.01s (+ 1.43%)0.84s0.86s
Check Time4.42s (± 0.44%)4.47s (± 0.42%)+0.05s (+ 1.15%)4.43s4.51s
Emit Time5.47s (± 1.35%)5.52s (± 0.68%)+0.05s (+ 0.91%)5.45s5.62s
Total Time12.30s (± 0.69%)12.43s (± 0.32%)+0.13s (+ 1.06%)12.34s12.53s
Monaco - node (v12.1.0, x64)
Memory used345,927k (± 0.01%)345,898k (± 0.01%)-29k (- 0.01%)345,827k346,030k
Parse Time1.23s (± 0.42%)1.22s (± 0.61%)-0.01s (- 0.73%)1.20s1.23s
Bind Time0.72s (± 1.15%)0.72s (± 0.65%)-0.00s (- 0.69%)0.71s0.73s
Check Time4.49s (± 0.27%)4.49s (± 0.56%)-0.01s (- 0.18%)4.43s4.54s
Emit Time2.98s (± 1.31%)2.98s (± 1.07%)+0.00s (+ 0.07%)2.93s3.09s
Total Time9.43s (± 0.42%)9.41s (± 0.50%)-0.02s (- 0.18%)9.34s9.53s
TFS - node (v12.1.0, x64)
Memory used304,312k (± 0.02%)304,352k (± 0.02%)+40k (+ 0.01%)304,231k304,476k
Parse Time0.95s (± 0.71%)0.95s (± 1.00%)+0.00s (+ 0.00%)0.92s0.96s
Bind Time0.68s (± 0.69%)0.68s (± 0.33%)-0.00s (- 0.15%)0.67s0.68s
Check Time4.05s (± 0.63%)4.05s (± 0.44%)+0.01s (+ 0.17%)4.01s4.09s
Emit Time3.05s (± 1.01%)3.07s (± 0.46%)+0.02s (+ 0.69%)3.04s3.10s
Total Time8.72s (± 0.50%)8.75s (± 0.23%)+0.03s (+ 0.33%)8.70s8.80s
Angular - node (v8.9.0, x64)
Memory used349,620k (± 0.01%)351,274k (± 0.01%)+1,654k (+ 0.47%)351,166k351,312k
Parse Time2.10s (± 0.40%)2.11s (± 0.36%)+0.01s (+ 0.62%)2.10s2.13s
Bind Time0.91s (± 0.71%)0.92s (± 0.79%)+0.01s (+ 1.54%)0.91s0.94s
Check Time5.29s (± 0.61%)5.28s (± 0.54%)-0.00s (- 0.08%)5.23s5.35s
Emit Time6.23s (± 1.08%)6.26s (± 0.71%)+0.03s (+ 0.55%)6.15s6.37s
Total Time14.52s (± 0.69%)14.58s (± 0.36%)+0.06s (+ 0.43%)14.48s14.72s
Monaco - node (v8.9.0, x64)
Memory used364,019k (± 0.01%)364,016k (± 0.01%)-3k (- 0.00%)363,893k364,129k
Parse Time1.56s (± 0.48%)1.56s (± 0.31%)0.00s ( 0.00%)1.55s1.57s
Bind Time0.93s (± 0.78%)0.93s (± 0.70%)0.00s ( 0.00%)0.91s0.94s
Check Time5.57s (± 0.59%)5.56s (± 0.44%)-0.01s (- 0.13%)5.50s5.60s
Emit Time3.03s (± 0.62%)3.04s (± 0.84%)+0.01s (+ 0.20%)3.00s3.11s
Total Time11.09s (± 0.41%)11.09s (± 0.37%)-0.00s (- 0.04%)11.04s11.20s
TFS - node (v8.9.0, x64)
Memory used320,953k (± 0.01%)320,948k (± 0.02%)-5k (- 0.00%)320,823k321,050k
Parse Time1.26s (± 0.51%)1.26s (± 0.29%)+0.00s (+ 0.16%)1.26s1.27s
Bind Time0.74s (± 0.60%)0.74s (± 0.75%)-0.00s (- 0.00%)0.73s0.75s
Check Time4.75s (± 0.40%)4.73s (± 0.54%)-0.02s (- 0.34%)4.69s4.81s
Emit Time3.19s (± 0.95%)3.19s (± 0.45%)+0.00s (+ 0.03%)3.17s3.23s
Total Time9.94s (± 0.48%)9.93s (± 0.39%)-0.02s (- 0.15%)9.86s10.02s
Angular - node (v8.9.0, x86)
Memory used198,592k (± 0.02%)199,462k (± 0.02%)+870k (+ 0.44%)199,388k199,511k
Parse Time2.03s (± 0.90%)2.04s (± 0.39%)+0.01s (+ 0.64%)2.02s2.06s
Bind Time1.04s (± 1.19%)1.01s (± 0.59%)-0.03s (- 2.68%)1.00s1.03s
Check Time4.81s (± 0.38%)4.80s (± 0.52%)-0.00s (- 0.04%)4.77s4.88s
Emit Time6.01s (± 0.79%)6.00s (± 0.46%)-0.02s (- 0.30%)5.94s6.05s
Total Time13.89s (± 0.46%)13.86s (± 0.19%)-0.03s (- 0.24%)13.81s13.93s
Monaco - node (v8.9.0, x86)
Memory used203,931k (± 0.02%)203,943k (± 0.03%)+13k (+ 0.01%)203,827k204,076k
Parse Time1.61s (± 0.90%)1.62s (± 1.12%)+0.01s (+ 0.37%)1.59s1.66s
Bind Time0.74s (± 0.87%)0.74s (± 0.80%)+0.00s (+ 0.27%)0.73s0.76s
Check Time5.40s (± 0.84%)5.39s (± 1.19%)-0.02s (- 0.31%)5.15s5.48s
Emit Time2.88s (± 2.58%)2.89s (± 2.67%)+0.00s (+ 0.14%)2.81s3.18s
Total Time10.65s (± 0.46%)10.64s (± 0.49%)-0.01s (- 0.06%)10.53s10.74s
TFS - node (v8.9.0, x86)
Memory used180,831k (± 0.03%)180,869k (± 0.02%)+38k (+ 0.02%)180,771k180,947k
Parse Time1.31s (± 0.85%)1.30s (± 0.81%)-0.00s (- 0.23%)1.28s1.33s
Bind Time0.69s (± 0.99%)0.70s (± 0.68%)+0.00s (+ 0.29%)0.69s0.71s
Check Time4.50s (± 0.74%)4.47s (± 0.65%)-0.03s (- 0.60%)4.40s4.55s
Emit Time2.96s (± 0.71%)2.97s (± 1.53%)+0.01s (+ 0.27%)2.85s3.09s
Total Time9.46s (± 0.63%)9.44s (± 0.78%)-0.02s (- 0.23%)9.33s9.63s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-166-generic
Architecturex64
Available Memory16 GB
Available Memory6 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)
BenchmarkNameIterations
Current3551310
Baselinemaster10

@typescript-bot

Copy link
Copy Markdown
Contributor

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
Copy Markdown
MemberAuthor

RWC and Community test errors look to be preexisting conditions.

@weswigham

Copy link
Copy Markdown
Member

RWC is clean on master. Maybe try syncing the branch and running again? Otherwise the change is real.

@ahejlsberg

Copy link
Copy Markdown
MemberAuthor

Looked at it more closely. The RWC difference is just simple error message change because we now pick a different (and arguably better) union constituent to elaborate.

(<TransientSymbol>prop).isDiscriminantProperty =
((<TransientSymbol>prop).checkFlags & CheckFlags.Discriminant) === CheckFlags.Discriminant &&
isDiscriminantType(getTypeOfSymbol(prop));
!maybeTypeOfKind(getTypeOfSymbol(prop), TypeFlags.Instantiable);

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.

What's the justification for not maybe instantiables here? Previously it was "not generic indexes", so this is quite a bit broader an exclusion, if I'm not mistaken.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

If you look at isGenericIndexType you'll see that it's more than just generic index types. The code change just clarifies what's actually going on, there's no change in behavior.

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, alright, so it's equivalent. Neat.

@weswighamWesley Wigham (weswigham) left a comment

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.

Do we also need to apply/extended similar logic to conditionals in some way? Like if I have a
(T extends U ? { type: "a" } : { type: "b" }) & { type: "b" }, shouldn't it be similarly simplified (at least outside of when infer variables are present)?

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression in 3.6: Combination of union and intersection types with literal types not resolving specific types properly

3 participants

@ahejlsberg@typescript-bot@weswigham