More accurately model OptionalMemberExpression
Extends the new modeling of the previous diff to OptionalMemberExpression. In an example such as `a?.b?.c`, we now model that not only is the `.c` conditional, but our control flow graph accurately reflects the fact that the `.c` is only evaluated if `a.b` exists. Previously we knew it was conditional but the CFG allowed a path from a being null through to evaluation of `.c`.
Joe Savona committed
May 2, 2023 at 15:44 UTC
c46157b00bbf163910e67a6695672a7ba05498a3
2 files changed
+163
-21
compiler/forget/src/HIR/BuildHIR.ts
+123
-9
@@ -1728,12 +1728,111 @@ function lowerExpression(
1728
}
1729
}
1730
1731
+function lowerOptionalMemberExpression(
1732
+ builder: HIRBuilder,
1733
+ expr: NodePath<t.OptionalMemberExpression>,
1734
+ parentAlternate: BlockId | null
1735
+): { object: Place; value: Place } {
1736
+ const optional = expr.node.optional;
1737
+ const loc = expr.node.loc ?? GeneratedSource;
1738
+ const place = buildTemporaryPlace(builder, loc);
1739
+ const continuationBlock = builder.reserve(builder.currentBlockKind());
1740
+ const consequent = builder.reserve("value");
1741
+
1742
+ // block to evaluate if the callee is null/undefined, this sets the result of the call to undefined.
1743
+ // note that we only create an alternate when first entering an optional subtree of the ast: if this
1744
+ // is a child of an optional node, we use the alterate created by the parent.
1745
+ const alternate =
1746
+ parentAlternate !== null
1747
+ ? parentAlternate
1748
+ : builder.enter("value", () => {
1749
+ const temp = lowerValueToTemporary(builder, {
1750
+ kind: "Primitive",
1751
+ value: undefined,
1752
+ loc,
1753
+ });
1754
+ lowerValueToTemporary(builder, {
1755
+ kind: "StoreLocal",
1756
+ lvalue: { kind: InstructionKind.Const, place: { ...place } },
1757
+ value: { ...temp },
1758
+ loc,
1759
+ });
1760
+ return {
1761
+ kind: "goto",
1762
+ variant: GotoVariant.Break,
1763
+ block: continuationBlock.id,
1764
+ id: makeInstructionId(0),
1765
+ loc,
1766
+ };
1767
+ });
1768
+
1769
+ let object: Place | null = null;
1770
+ const testBlock = builder.enter("value", () => {
1771
+ const objectPath = expr.get("object");
1772
+ if (objectPath.isOptionalMemberExpression()) {
1773
+ const { value } = lowerOptionalMemberExpression(
1774
+ builder,
1775
+ objectPath,
1776
+ alternate
1777
+ );
1778
+ object = value;
1779
+ } else if (objectPath.isOptionalCallExpression()) {
1780
+ const value = lowerOptionalCallExpression(builder, objectPath, alternate);
1781
+ object = lowerValueToTemporary(builder, value);
1782
+ } else {
1783
+ object = lowerExpressionToTemporary(builder, objectPath);
1784
+ }
1785
+ return {
1786
+ kind: "branch",
1787
+ test: { ...object },
1788
+ consequent: consequent.id,
1789
+ alternate,
1790
+ id: makeInstructionId(0),
1791
+ loc,
1792
+ };
1793
+ });
1794
+ invariant(object !== null, "Satisfy type checker");
1795
+
1796
+ // block to evaluate if the callee is non-null/undefined. arguments are lowered in this block to preserve
1797
+ // the semantic of conditional evaluation depending on the callee
1798
+ builder.enterReserved(consequent, () => {
1799
+ const { value } = lowerMemberExpression(builder, expr, object);
1800
+ const temp = lowerValueToTemporary(builder, value);
1801
+ lowerValueToTemporary(builder, {
1802
+ kind: "StoreLocal",
1803
+ lvalue: { kind: InstructionKind.Const, place: { ...place } },
1804
+ value: { ...temp },
1805
+ loc,
1806
+ });
1807
+ return {
1808
+ kind: "goto",
1809
+ variant: GotoVariant.Break,
1810
+ block: continuationBlock.id,
1811
+ id: makeInstructionId(0),
1812
+ loc,
1813
+ };
1814
+ });
1815
+
1816
+ builder.terminateWithContinuation(
1817
+ {
1818
+ kind: "optional-call",
1819
+ optional,
1820
+ test: testBlock,
1821
+ fallthrough: continuationBlock.id,
1822
+ id: makeInstructionId(0),
1823
+ loc,
1824
+ },
1825
+ continuationBlock
1826
+ );
1827
+
1828
+ return { object, value: place };
1829
+}
1830
+
1831
function lowerOptionalCallExpression(
1832
builder: HIRBuilder,
1733
- exprPath: NodePath<t.OptionalCallExpression>,
1833
+ expr: NodePath<t.OptionalCallExpression>,
1834
parentAlternate: BlockId | null
1835
): InstructionValue {
1736
- const expr = exprPath as NodePath<t.OptionalCallExpression>;
1836
const optional = expr.node.optional;
1837
const calleePath = expr.get("callee");
1838
const loc = expr.node.loc ?? GeneratedSource;
@@ -1782,10 +1881,18 @@ function lowerOptionalCallExpression(
1881
kind: "CallExpression",
1882
callee: valuePlace,
1883
};
1785
- } else if (
1786
- calleePath.isMemberExpression() ||
1787
- calleePath.isOptionalMemberExpression()
1788
- ) {
1884
+ } else if (calleePath.isOptionalMemberExpression()) {
1885
+ const { object, value } = lowerOptionalMemberExpression(
1886
+ builder,
1887
+ calleePath,
1888
+ alternate
1889
+ );
1890
+ callee = {
1891
+ kind: "MethodCall",
1892
+ receiver: object,
1893
+ property: value,
1894
+ };
1895
+ } else if (calleePath.isMemberExpression()) {
1896
const memberExpr = lowerMemberExpression(builder, calleePath);
1897
const propertyPlace = lowerValueToTemporary(builder, memberExpr.value);
1898
callee = {
@@ -1985,15 +2092,22 @@ function lowerArguments(
2092
return args;
2093
}
2094
2095
+type LoweredMemberExpression = {
2096
+ object: Place;
2097
+ property: Place | string;
2098
+ value: InstructionValue;
2099
+};
2100
function lowerMemberExpression(
2101
builder: HIRBuilder,
1990
- expr: NodePath<t.MemberExpression | t.OptionalMemberExpression>
1991
-): { object: Place; property: Place | string; value: InstructionValue } {
2102
+ expr: NodePath<t.MemberExpression | t.OptionalMemberExpression>,
2103
+ loweredObject: Place | null = null
2104
+): LoweredMemberExpression {
2105
const exprNode = expr.node;
2106
const exprLoc = exprNode.loc ?? GeneratedSource;
2107
const objectNode = expr.get("object");
2108
const propertyNode = expr.get("property");
1996
- const object = lowerExpressionToTemporary(builder, objectNode);
2109
+ const object =
2110
+ loweredObject ?? lowerExpressionToTemporary(builder, objectNode);
2111
2112
if (
2113
objectNode.isOptionalMemberExpression() &&
compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts
+40
-12
@@ -685,17 +685,44 @@ function codegenInstructionValue(
685
break;
686
}
687
case "OptionalCall": {
688
- const call = codegenInstructionValue(cx, instrValue.call);
689
- invariant(call.type === "CallExpression", "Expected a call expression");
690
- invariant(
691
- t.isExpression(call.callee),
692
- "v8 intrinsics are validated during lowering"
693
- );
694
- value = t.optionalCallExpression(
695
- call.callee,
696
- call.arguments,
697
- instrValue.optional
698
- );
688
+ const optionalValue = codegenInstructionValue(cx, instrValue.call);
689
+ switch (optionalValue.type) {
690
+ case "OptionalCallExpression":
691
+ case "CallExpression": {
692
+ invariant(
693
+ t.isExpression(optionalValue.callee),
694
+ "v8 intrinsics are validated during lowering"
695
+ );
696
+ value = t.optionalCallExpression(
697
+ optionalValue.callee,
698
+ optionalValue.arguments,
699
+ instrValue.optional
700
+ );
701
+ break;
702
+ }
703
+ case "OptionalMemberExpression":
704
+ case "MemberExpression": {
705
+ const property = optionalValue.property;
706
+ invariant(
707
+ t.isExpression(property),
708
+ "Private names are validated during lowering"
709
+ );
710
+ value = t.optionalMemberExpression(
711
+ optionalValue.object,
712
+ property,
713
+ optionalValue.computed,
714
+ instrValue.optional
715
+ );
716
+ break;
717
+ }
718
+ default: {
719
+ CompilerError.invariant(
720
+ "Expected an optional value to resolve to a call expression or member expression",
721
+ instrValue.loc,
722
+ `Got a '${optionalValue.type}'`
723
+ );
724
+ }
725
+ }
726
break;
727
}
728
case "MethodCall": {
@@ -703,7 +730,8 @@ function codegenInstructionValue(
730
invariant(
731
t.isMemberExpression(memberExpr) ||
732
t.isOptionalMemberExpression(memberExpr),
706
- "[Codegen] Internal error: MethodCall::property must be an unpromoted + unmemoized MemberExpression."
733
+ "[Codegen] Internal error: MethodCall::property must be an unpromoted + unmemoized MemberExpression. " +
734
+ `Got a '${memberExpr.type}'`
735
);
736
invariant(
737
t.isNodesEquivalent(