Fix switch statement exhaustiveness checking #34840
Conversation
|
@typescript-bot perf test this |
|
Heya @ahejlsberg, I've started to run the perf test suite on this PR at 576e5f5. You can monitor the build here. It should now contribute to this PR's status checks. |
|
@typescript-bot perf test this |
|
Heya @ahejlsberg, I've started to run the perf test suite on this PR at 576e5f5. You can monitor the build here. It should now contribute to this PR's status checks. |
|
@typescript-bot perf test this |
|
Heya @ahejlsberg, I've started to run the perf test suite on this PR at 576e5f5. You can monitor the build here. It should now contribute to this PR's status checks. |
|
@typescript-bot perf test this |
|
Heya @ahejlsberg, I've started to run the perf test suite on this PR at 576e5f5. You can monitor the build here. It should now contribute to this PR's status checks. |
|
@typescript-bot perf test this |
|
Heya @rbuckton, I've started to run the perf test suite on this PR at 576e5f5. You can monitor the build here. It should now contribute to this PR's status checks. Update: The results are in! |
|
@rbuckton Here they are:Comparison Report - master..34840
System
Hosts
Scenarios
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@ahejlsberg this PR has broken our nightly publish (you can verify by checking out this branch and running in this code: private _getLinks(node: DirectoryInode) {
if (!node.links) {
const links = new collections.SortedMap<string, Inode>(this.stringComparer);
const { source, resolver } = node;
if (source && resolver) {
node.source = undefined;
node.resolver = undefined;
for (const name of resolver.readdirSync(source)) {
const path = vpath.combine(source, name);
const stats = resolver.statSync(path); // error on this variable declaration
switch (stats.mode & S_IFMT) {
case S_IFDIR:
const dir = this._mknod(node.dev, S_IFDIR, 0o777);
dir.source = vpath.combine(source, name);
dir.resolver = resolver;
this._addLink(node, links, name, dir);
break;
case S_IFREG:
const file = this._mknod(node.dev, S_IFREG, 0o666);
file.source = vpath.combine(source, name);
file.resolver = resolver;
file.size = stats.size;
this._addLink(node, links, name, file);
break;
}
}
}
else if (this._shadowRoot && node.shadowRoot) {
this._copyShadowLinks(this._shadowRoot._getLinks(node.shadowRoot), links);
}
node.links = links;
}
return node.links;
}we appear to be "finding" a completely bogus circularity. How would you like to handle it? |
|
The root issue here is that we're a bit too aggressive in the exhaustiveness analysis. Specifically, when analyzing the possible control flows in a function foo() {
const foo: number | undefined = 0;
while (true) {
const stats = foo; // Circularity error here!
switch (stats) {
case 1: break;
case 2: break;
}
}
}To determine the type of A better approach would be to separately determine the type coming from the non-bypass control flows and the type coming from the bypass flow. Then, only if the type resulting from the bypass flow would widen the final type do we need to check whether to actually include it (i.e. only then do we need the exhaustiveness check). I'll work on a fix to implement that strategy. |
|
Does this fix this code, too? type JsonDataTypeNames = 'string' | 'number' | 'object' | 'array' | 'boolean' | 'null' | 'integer';
declare var schemaType: JsonDataTypeNames | JsonDataTypeNames[] | undefined;
if (!Array.isArray(schemaType)) {
} else {
for (let i = 0, ii = schemaType.length; i < ii; i++) {
let validType = schemaType[i]; // any in ts3.7! not JsonDataTypeNames as in 3.6
switch (validType) { }
}
} |
|
@HolgerJeromin It does indeed! |
microsoft/TypeScript#34840 has release in 3.8.2
Fixes #34661.