Restrict switch case test values until they use value blocks
We currently lower switch case test values within the wrong scope: the test value really should be a value block rather than a `Place`. Until then, this PR adds a bailout for complex test values: we allow primitives and identifiers which should cover most real-world use-cases.
Joe Savona committed
Feb 2, 2023 at 09:09 UTC
34a7b58334011eace13767dc32c37a5336819983
5 files changed
+90
-19
compiler/forget/src/HIR/BuildHIR.ts
+29
-6
@@ -461,8 +461,8 @@ function lowerStatement(
461
let hasDefault = false;
462
for (let ii = stmt.get("cases").length - 1; ii >= 0; ii--) {
463
const case_: NodePath<t.SwitchCase> = stmt.get("cases")[ii];
464
- const test = case_.get("test");
465
- if (test.node == null) {
464
+ const testExpr = case_.get("test");
465
+ if (testExpr.node == null) {
466
if (hasDefault) {
467
builder.errors.push({
468
reason:
@@ -491,11 +491,34 @@ function lowerStatement(
491
};
492
});
493
});
494
+ let test: Place | null = null;
495
+ if (testExpr.node != null) {
496
+ switch (testExpr.node.type) {
497
+ case "Identifier":
498
+ case "StringLiteral":
499
+ case "NumericLiteral":
500
+ case "NullLiteral":
501
+ case "BooleanLiteral":
502
+ case "BigIntLiteral": {
503
+ // ok
504
+ break;
505
+ }
506
+ default: {
507
+ builder.errors.push({
508
+ reason:
509
+ "(BuildHIR::lowerStatement) Switch case test values must be identifiers or primitives, compound values are not yet supported",
510
+ severity: ErrorSeverity.Todo,
511
+ nodePath: testExpr,
512
+ });
513
+ }
514
+ }
515
+ test = lowerExpressionToPlace(
516
+ builder,
517
+ testExpr as NodePath<t.Expression>
518
+ );
519
+ }
520
cases.push({
495
- test:
496
- test.node != null
497
- ? lowerExpressionToPlace(builder, test as NodePath<t.Expression>)
498
- : null,
521
+ test,
522
block,
523
});
524
fallthrough = block;
compiler/forget/src/__tests__/fixtures/hir/error.todo-kitchensink.expect.md
+44
-5
@@ -58,6 +58,17 @@ function foo([a, b], { c, d, e = "e" }, f = "f", ...args) {
58
++updateIdentifier;
59
updateIdentifier.y++;
60
updateIdentifier.y--;
61
+
62
+ switch (i) {
63
+ case 1 + 1: {
64
+ }
65
+ case foo(): {
66
+ }
67
+ case x.y: {
68
+ }
69
+ default: {
70
+ }
71
+ }
72
}
73
74
```
@@ -353,7 +364,7 @@ function foo([a, b], { c, d, e = "e" }, f = "f", ...args) {
364
| ^^^^^^^^^^^^^^^^^^
365
55 | updateIdentifier.y++;
366
56 | updateIdentifier.y--;
356
- 57 | }
367
+ 57 |
368
369
[ReactForget] TodoError: (BuildHIR::lowerExpression) Handle UpdateExpression with MemberExpression argument
370
53 | --updateIdentifier;
@@ -361,16 +372,44 @@ function foo([a, b], { c, d, e = "e" }, f = "f", ...args) {
372
> 55 | updateIdentifier.y++;
373
| ^^^^^^^^^^^^^^^^^^^^
374
56 | updateIdentifier.y--;
364
- 57 | }
365
- 58 |
375
+ 57 |
376
+ 58 | switch (i) {
377
378
[ReactForget] TodoError: (BuildHIR::lowerExpression) Handle UpdateExpression with MemberExpression argument
379
54 | ++updateIdentifier;
380
55 | updateIdentifier.y++;
381
> 56 | updateIdentifier.y--;
382
| ^^^^^^^^^^^^^^^^^^^^
372
- 57 | }
373
- 58 |
383
+ 57 |
384
+ 58 | switch (i) {
385
+ 59 | case 1 + 1: {
386
+
387
+[ReactForget] TodoError: (BuildHIR::lowerStatement) Switch case test values must be identifiers or primitives, compound values are not yet supported
388
+ 61 | case foo(): {
389
+ 62 | }
390
+> 63 | case x.y: {
391
+ | ^^^
392
+ 64 | }
393
+ 65 | default: {
394
+ 66 | }
395
+
396
+[ReactForget] TodoError: (BuildHIR::lowerStatement) Switch case test values must be identifiers or primitives, compound values are not yet supported
397
+ 59 | case 1 + 1: {
398
+ 60 | }
399
+> 61 | case foo(): {
400
+ | ^^^^^
401
+ 62 | }
402
+ 63 | case x.y: {
403
+ 64 | }
404
+
405
+[ReactForget] TodoError: (BuildHIR::lowerStatement) Switch case test values must be identifiers or primitives, compound values are not yet supported
406
+ 57 |
407
+ 58 | switch (i) {
408
+> 59 | case 1 + 1: {
409
+ | ^^^^^
410
+ 60 | }
411
+ 61 | case foo(): {
412
+ 62 | }
413
```
414
415
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/error.todo-kitchensink.js
+11
@@ -54,4 +54,15 @@ function foo([a, b], { c, d, e = "e" }, f = "f", ...args) {
54
++updateIdentifier;
55
updateIdentifier.y++;
56
updateIdentifier.y--;
57
+
58
+ switch (i) {
59
+ case 1 + 1: {
60
+ }
61
+ case foo(): {
62
+ }
63
+ case x.y: {
64
+ }
65
+ default: {
66
+ }
67
+ }
68
}
compiler/forget/src/__tests__/fixtures/hir/ssa-switch.expect.md
+4
-6
@@ -6,11 +6,11 @@ function foo() {
6
let x = 1;
7
8
switch (x) {
9
- case x === 1: {
9
+ case 1: {
10
x = x + 1;
11
break;
12
}
13
- case x === 2: {
13
+ case 2: {
14
x = x + 2;
15
break;
16
}
@@ -30,20 +30,18 @@ function foo() {
30
function foo() {
31
const $ = React.useMemoCache();
32
const x = 1;
33
- 2;
34
- 1;
33
let x$0;
34
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
35
x$0 = undefined;
36
bb1: switch (x) {
39
- case true: {
37
+ case 1: {
38
1;
39
40
const x$1 = 2;
41
x$0 = x$1;
42
break bb1;
43
}
46
- case false: {
44
+ case 2: {
45
2;
46
47
const x$2 = 3;
compiler/forget/src/__tests__/fixtures/hir/ssa-switch.js
+2
-2
@@ -2,11 +2,11 @@ function foo() {
2
let x = 1;
3
4
switch (x) {
5
- case x === 1: {
5
+ case 1: {
6
x = x + 1;
7
break;
8
}
9
- case x === 2: {
9
+ case 2: {
10
x = x + 2;
11
break;
12
}