@samitouri / QOS-React-2 / commits / a9d6d2d95a

[rfc] always create collections even if elements have errors

BuildHIR currently propagates UnsupportedNodes for collection types where the element itself can fail (for example object expressions where the key may not be valid). However, given that we currently abort compilation after the first failing pass (and will probably do so for quite a while) I think we can simplify and just always return the collection. Note that I already did this for destructuring. I'm open to leaving the code as-is if you prefer, though.

Joe Savona committed Mar 6, 2023 at 14:43 UTC a9d6d2d95ac567d0d18967b8ce0eb3429d24401d
1 file changed +29 -55
compiler/forget/src/HIR/BuildHIR.ts
+29 -55
@@ -799,7 +799,6 @@ function lowerExpression(
799 const expr = exprPath as NodePath<t.ObjectExpression>;
800 const propertyPaths = expr.get("properties");
801 const properties: Map<string, Place> = new Map();
802 - let hasError = false;
802 for (const propertyPath of propertyPaths) {
803 if (!propertyPath.isObjectProperty()) {
804 builder.errors.push({
@@ -807,7 +806,6 @@ function lowerExpression(
806 severity: ErrorSeverity.Todo,
807 nodePath: propertyPath,
808 });
810 - hasError = true;
809 continue;
810 }
811 const key = propertyPath.node.key;
@@ -817,7 +815,6 @@ function lowerExpression(
815 severity: ErrorSeverity.InvalidInput,
816 nodePath: propertyPath,
817 });
820 - hasError = true;
818 continue;
819 }
820 const valuePath = propertyPath.get("value");
@@ -827,23 +824,19 @@ function lowerExpression(
824 severity: ErrorSeverity.Todo,
825 nodePath: valuePath,
826 });
830 - hasError = true;
827 continue;
828 }
829 const value = lowerExpressionToTemporary(builder, valuePath);
830 properties.set(key.name, value);
831 }
836 - return hasError
837 - ? { kind: "UnsupportedNode", node: exprNode, loc: exprLoc }
838 - : {
839 - kind: "ObjectExpression",
840 - properties,
841 - loc: exprLoc,
842 - };
832 + return {
833 + kind: "ObjectExpression",
834 + properties,
835 + loc: exprLoc,
836 + };
837 }
838 case "ArrayExpression": {
839 const expr = exprPath as NodePath<t.ArrayExpression>;
846 - let hasError = false;
840 let elements: Place[] = [];
841 for (const element of expr.get("elements")) {
842 if (element.node == null || !element.isExpression()) {
@@ -852,20 +845,17 @@ function lowerExpression(
845 severity: ErrorSeverity.Todo,
846 nodePath: element,
847 });
855 - hasError = true;
848 continue;
849 }
850 elements.push(
851 lowerExpressionToTemporary(builder, element as NodePath<t.Expression>)
852 );
853 }
862 - return hasError
863 - ? { kind: "UnsupportedNode", node: exprNode, loc: exprLoc }
864 - : {
865 - kind: "ArrayExpression",
866 - elements,
867 - loc: exprLoc,
868 - };
854 + return {
855 + kind: "ArrayExpression",
856 + elements,
857 + loc: exprLoc,
858 + };
859 }
860 case "NewExpression": {
861 const expr = exprPath as NodePath<t.NewExpression>;
@@ -880,7 +870,6 @@ function lowerExpression(
870 }
871 const callee = lowerExpressionToTemporary(builder, calleePath);
872 let args: Place[] = [];
883 - let hasError = false;
873 for (const argPath of expr.get("arguments")) {
874 if (!argPath.isExpression()) {
875 builder.errors.push({
@@ -888,25 +877,21 @@ function lowerExpression(
877 severity: ErrorSeverity.Todo,
878 nodePath: argPath,
879 });
891 - hasError = true;
880 continue;
881 }
882 args.push(lowerExpressionToTemporary(builder, argPath));
883 }
884
897 - return hasError
898 - ? { kind: "UnsupportedNode", node: exprNode, loc: exprLoc }
899 - : {
900 - kind: "NewExpression",
901 - callee,
902 - args,
903 - loc: exprLoc,
904 - };
885 + return {
886 + kind: "NewExpression",
887 + callee,
888 + args,
889 + loc: exprLoc,
890 + };
891 }
892 case "CallExpression": {
893 const expr = exprPath as NodePath<t.CallExpression>;
894 const calleePath = expr.get("callee");
909 - let hasError = false;
895 if (!calleePath.isExpression()) {
896 builder.errors.push({
897 reason: `(BuildHIR::lowerExpression) Expected Expression, got ${calleePath.type} in CallExpression (v8 intrinsics not supported)`,
@@ -925,7 +910,6 @@ function lowerExpression(
910 severity: ErrorSeverity.Todo,
911 nodePath: argPath,
912 });
928 - hasError = true;
913 continue;
914 }
915 args.push(lowerExpressionToTemporary(builder, argPath));
@@ -957,19 +941,16 @@ function lowerExpression(
941 severity: ErrorSeverity.Todo,
942 nodePath: argPath,
943 });
960 - hasError = true;
944 continue;
945 }
946 args.push(lowerExpressionToTemporary(builder, argPath));
947 }
965 - return hasError
966 - ? { kind: "UnsupportedNode", node: exprNode, loc: exprLoc }
967 - : {
968 - kind: "CallExpression",
969 - callee,
970 - args,
971 - loc: exprLoc,
972 - };
948 + return {
949 + kind: "CallExpression",
950 + callee,
951 + args,
952 + loc: exprLoc,
953 + };
954 }
955 }
956 case "BinaryExpression": {
@@ -1359,7 +1340,6 @@ function lowerExpression(
1340 .get("children")
1341 .map((child) => lowerJsxElement(builder, child));
1342 const props: Array<JsxAttribute> = [];
1362 - let hasError = false;
1343 for (const attribute of opening.get("attributes")) {
1344 if (attribute.isJSXSpreadAttribute()) {
1345 const argument = lowerExpressionToTemporary(
@@ -1375,7 +1355,6 @@ function lowerExpression(
1355 severity: ErrorSeverity.Todo,
1356 nodePath: attribute,
1357 });
1378 - hasError = true;
1358 continue;
1359 }
1360 const name = attribute.get("name");
@@ -1385,7 +1364,6 @@ function lowerExpression(
1364 severity: ErrorSeverity.Todo,
1365 nodePath: name,
1366 });
1388 - hasError = true;
1367 continue;
1368 }
1369 const valueExpr = attribute.get("value");
@@ -1399,7 +1377,6 @@ function lowerExpression(
1377 severity: ErrorSeverity.Todo,
1378 nodePath: valueExpr,
1379 });
1402 - hasError = true;
1380 continue;
1381 }
1382 const expression = valueExpr.get("expression");
@@ -1409,7 +1386,6 @@ function lowerExpression(
1386 severity: ErrorSeverity.Todo,
1387 nodePath: valueExpr,
1388 });
1412 - hasError = true;
1389 continue;
1390 }
1391 value = lowerExpressionToTemporary(builder, expression);
@@ -1417,15 +1393,13 @@ function lowerExpression(
1393 const prop: string = name.node.name;
1394 props.push({ kind: "JsxAttribute", name: prop, place: value });
1395 }
1420 - return hasError
1421 - ? { kind: "UnsupportedNode", node: exprNode, loc: exprLoc }
1422 - : {
1423 - kind: "JsxExpression",
1424 - tag,
1425 - props,
1426 - children,
1427 - loc: exprLoc,
1428 - };
1396 + return {
1397 + kind: "JsxExpression",
1398 + tag,
1399 + props,
1400 + children,
1401 + loc: exprLoc,
1402 + };
1403 }
1404 case "JSXFragment": {
1405 const expr = exprPath as NodePath<t.JSXFragment>;