@samitouri / QOS-React-1 / commits / afbaa8d3ca

Add a reason to ValueKind for better error messages (#2447)

Jan Kassens committed Dec 15, 2023 at 10:20 UTC afbaa8d3caf2bd2be72b5c5dbe2280808b4c4b20
13 files changed +376 -143
compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts
+27
@@ -942,6 +942,33 @@ export type Identifier = {
942 type: Type;
943 };
944
945 +export type AbstractValue = {
946 + kind: ValueKind;
947 + reason: ReadonlySet<ValueReason>;
948 +};
949 +
950 +/**
951 + * The reason for the kind of a value.
952 + */
953 +export enum ValueReason {
954 + /**
955 + * Defined outside the React function.
956 + */
957 + Global = "global",
958 +
959 + /**
960 + * Used in a JSX expression.
961 + */
962 + JsxCaptured = "jsx-captured",
963 +
964 + /**
965 + * Return value of a function with known frozen return value, e.g. `useState`.
966 + */
967 + KnownReturnSignature = "known-return-signature",
968 +
969 + Other = "other",
970 +}
971 +
972 /*
973 * Distinguish between different kinds of values relevant to inference purposes:
974 * see the main docblock for the module for details.
compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts
+337 -127
@@ -8,6 +8,7 @@
8 import { CompilerError } from "../CompilerError";
9 import { Environment } from "../HIR";
10 import {
11 + AbstractValue,
12 BasicBlock,
13 BlockId,
14 CallExpression,
@@ -17,13 +18,14 @@ import {
18 IdentifierId,
19 InstructionKind,
20 InstructionValue,
20 - isMutableEffect,
21 - isObjectType,
21 MethodCall,
22 Phi,
23 Place,
24 Type,
25 ValueKind,
26 + ValueReason,
27 + isMutableEffect,
28 + isObjectType,
29 } from "../HIR/HIR";
30 import { FunctionSignature } from "../HIR/ObjectShape";
31 import {
@@ -103,7 +105,10 @@ export default function inferReferenceEffects(
105 loc: fn.loc,
106 value: undefined,
107 };
106 - initialState.initialize(value, ValueKind.Frozen);
108 + initialState.initialize(value, {
109 + kind: ValueKind.Frozen,
110 + reason: new Set([ValueReason.Other]),
111 + });
112
113 for (const ref of fn.context) {
114 // TODO(gsn): This is a hack.
@@ -112,13 +117,22 @@ export default function inferReferenceEffects(
117 properties: [],
118 loc: ref.loc,
119 };
115 - initialState.initialize(value, ValueKind.Context);
120 + initialState.initialize(value, {
121 + kind: ValueKind.Context,
122 + reason: new Set([ValueReason.Other]),
123 + });
124 initialState.define(ref, value);
125 }
126
119 - const paramKind = options.isFunctionExpression
120 - ? ValueKind.Mutable
121 - : ValueKind.Frozen;
127 + const paramKind: AbstractValue = options.isFunctionExpression
128 + ? {
129 + kind: ValueKind.Mutable,
130 + reason: new Set([ValueReason.Other]),
131 + }
132 + : {
133 + kind: ValueKind.Frozen,
134 + reason: new Set([ValueReason.Other]),
135 + };
136 for (const param of fn.params) {
137 let value: InstructionValue;
138 let place: Place;
@@ -194,7 +208,7 @@ class InferenceState {
208 #env: Environment;
209
210 // The kind of reach value, based on its allocation site
197 - #values: Map<InstructionValue, ValueKind>;
211 + #values: Map<InstructionValue, AbstractValue>;
212 /*
213 * The set of values pointed to by each identifier. This is a set
214 * to accomodate phi points (where a variable may have different
@@ -204,7 +218,7 @@ class InferenceState {
218
219 constructor(
220 env: Environment,
207 - values: Map<InstructionValue, ValueKind>,
221 + values: Map<InstructionValue, AbstractValue>,
222 variables: Map<IdentifierId, Set<InstructionValue>>
223 ) {
224 this.#env = env;
@@ -217,7 +231,7 @@ class InferenceState {
231 }
232
233 // (Re)initializes a @param value with its default @param kind.
220 - initialize(value: InstructionValue, kind: ValueKind): void {
234 + initialize(value: InstructionValue, kind: AbstractValue): void {
235 CompilerError.invariant(value.kind !== "LoadLocal", {
236 reason:
237 "Expected all top-level identifiers to be defined as variables, not values",
@@ -240,7 +254,7 @@ class InferenceState {
254 }
255
256 // Lookup the kind of the given @param value.
243 - kind(place: Place): ValueKind {
257 + kind(place: Place): AbstractValue {
258 const values = this.#variables.get(place.identifier.id);
259 CompilerError.invariant(values != null, {
260 reason: `[hoisting] Expected value kind to be initialized`,
@@ -248,10 +262,11 @@ class InferenceState {
262 loc: place.loc,
263 suggestions: null,
264 });
251 - let mergedKind: ValueKind | null = null;
265 + let mergedKind: AbstractValue | null = null;
266 for (const value of values) {
267 const kind = this.#values.get(value)!;
254 - mergedKind = mergedKind !== null ? mergeValues(mergedKind, kind) : kind;
268 + mergedKind =
269 + mergedKind !== null ? mergeAbstractValues(mergedKind, kind) : kind;
270 }
271 CompilerError.invariant(mergedKind !== null, {
272 reason: `InferReferenceEffects::kind: Expected at least one value`,
@@ -303,7 +318,7 @@ class InferenceState {
318 * Similarly, a freeze reference is converted to readonly if the
319 * value is already frozen or is immutable.
320 */
306 - reference(place: Place, effectKind: Effect): void {
321 + reference(place: Place, effectKind: Effect, reason: ValueReason): void {
322 const values = this.#variables.get(place.identifier.id);
323 if (values === undefined) {
324 CompilerError.invariant(effectKind !== Effect.Store, {
@@ -318,24 +333,31 @@ class InferenceState {
333 : Effect.Read;
334 return;
335 }
321 - let valueKind: ValueKind | null = this.kind(place);
336 + let valueKind: AbstractValue | null = this.kind(place);
337 let effect: Effect | null = null;
338 switch (effectKind) {
339 case Effect.Freeze: {
340 if (
326 - valueKind === ValueKind.Mutable ||
327 - valueKind === ValueKind.Context ||
328 - valueKind === ValueKind.MaybeFrozen
341 + valueKind.kind === ValueKind.Mutable ||
342 + valueKind.kind === ValueKind.Context ||
343 + valueKind.kind === ValueKind.MaybeFrozen
344 ) {
345 + const reasonSet = new Set([reason]);
346 effect = Effect.Freeze;
331 - valueKind = ValueKind.Frozen;
347 + valueKind = {
348 + kind: ValueKind.Frozen,
349 + reason: reasonSet,
350 + };
351 values.forEach((value) => {
333 - this.#values.set(value, ValueKind.Frozen);
352 + this.#values.set(value, {
353 + kind: ValueKind.Frozen,
354 + reason: reasonSet,
355 + });
356
357 if (this.#env.config.enableTransitivelyFreezeFunctionExpressions) {
358 if (value.kind === "FunctionExpression") {
359 for (const operand of eachInstructionValueOperand(value)) {
338 - this.reference(operand, Effect.Freeze);
360 + this.reference(operand, Effect.Freeze, ValueReason.Other);
361 }
362 }
363 }
@@ -347,8 +369,8 @@ class InferenceState {
369 }
370 case Effect.ConditionallyMutate: {
371 if (
350 - valueKind === ValueKind.Mutable ||
351 - valueKind === ValueKind.Context
372 + valueKind.kind === ValueKind.Mutable ||
373 + valueKind.kind === ValueKind.Context
374 ) {
375 effect = Effect.ConditionallyMutate;
376 } else {
@@ -358,13 +380,14 @@ class InferenceState {
380 }
381 case Effect.Mutate: {
382 if (
361 - valueKind === ValueKind.Mutable ||
362 - valueKind === ValueKind.Context
383 + valueKind.kind === ValueKind.Mutable ||
384 + valueKind.kind === ValueKind.Context
385 ) {
386 effect = Effect.Mutate;
387 } else {
388 + let reason = getWriteErrorReason(valueKind);
389 CompilerError.throwInvalidReact({
367 - reason: `This mutates a global or a variable after it was passed to React, which means that React cannot observe changes to it`,
390 + reason,
391 description:
392 place.identifier.name !== null
393 ? `Found mutation of ${place.identifier.name}`
@@ -377,11 +400,13 @@ class InferenceState {
400 }
401 case Effect.Store: {
402 if (
380 - valueKind !== ValueKind.Mutable &&
381 - valueKind !== ValueKind.Context
403 + valueKind.kind !== ValueKind.Mutable &&
404 + valueKind.kind !== ValueKind.Context
405 ) {
406 + let reason = getWriteErrorReason(valueKind);
407 +
408 CompilerError.throwInvalidReact({
384 - reason: `This mutates a global or a variable after it was passed to React, which means that React cannot observe changes to it`,
409 + reason,
410 description:
411 place.identifier.name !== null
412 ? `Found mutation of ${place.identifier.name}`
@@ -395,7 +420,7 @@ class InferenceState {
420 * TODO(gsn): This should be bailout once we add bailout infra.
421 *
422 * invariant(
398 - * valueKind === ValueKind.Mutable,
423 + * valueKind.kind === ValueKindKind.Mutable,
424 * `expected valueKind to be 'Mutable' but found to be '${valueKind}'`
425 * );
426 */
@@ -404,9 +429,9 @@ class InferenceState {
429 }
430 case Effect.Capture: {
431 if (
407 - valueKind === ValueKind.Immutable ||
408 - valueKind === ValueKind.Frozen ||
409 - valueKind === ValueKind.MaybeFrozen
432 + valueKind.kind === ValueKind.Immutable ||
433 + valueKind.kind === ValueKind.Frozen ||
434 + valueKind.kind === ValueKind.MaybeFrozen
435 ) {
436 effect = Effect.Read;
437 } else {
@@ -456,13 +481,13 @@ class InferenceState {
481 * termination.
482 */
483 merge(other: InferenceState): InferenceState | null {
459 - let nextValues: Map<InstructionValue, ValueKind> | null = null;
484 + let nextValues: Map<InstructionValue, AbstractValue> | null = null;
485 let nextVariables: Map<IdentifierId, Set<InstructionValue>> | null = null;
486
487 for (const [id, thisValue] of this.#values) {
488 const otherValue = other.#values.get(id);
489 if (otherValue !== undefined) {
465 - const mergedValue = mergeValues(thisValue, otherValue);
490 + const mergedValue = mergeAbstractValues(thisValue, otherValue);
491 if (mergedValue !== thisValue) {
492 nextValues = nextValues ?? new Map(this.#values);
493 nextValues.set(id, mergedValue);
@@ -658,6 +683,33 @@ function mergeValues(a: ValueKind, b: ValueKind): ValueKind {
683 }
684 }
685
686 +/**
687 + * @returns `true` if `a` is a superset of `b`.
688 + */
689 +function isSuperset<T>(a: ReadonlySet<T>, b: ReadonlySet<T>): boolean {
690 + for (const v of b) {
691 + if (!a.has(v)) {
692 + return false;
693 + }
694 + }
695 + return true;
696 +}
697 +
698 +function mergeAbstractValues(
699 + a: AbstractValue,
700 + b: AbstractValue
701 +): AbstractValue {
702 + const kind = mergeValues(a.kind, b.kind);
703 + if (kind === a.kind && kind === b.kind && isSuperset(a.reason, b.reason)) {
704 + return a;
705 + }
706 + const reason = new Set(a.reason);
707 + for (const r of b.reason) {
708 + reason.add(r);
709 + }
710 + return { kind, reason };
711 +}
712 +
713 /*
714 * Iterates over the given @param block, defining variables and
715 * recording references on the @param state according to JS semantics.
@@ -673,47 +725,77 @@ function inferBlock(
725
726 for (const instr of block.instructions) {
727 const instrValue = instr.value;
676 - let effectKind: Effect | null = null;
728 + let effect: { kind: Effect; reason: ValueReason } | null = null;
729 let lvalueEffect = Effect.ConditionallyMutate;
678 - let valueKind: ValueKind;
730 + let valueKind: AbstractValue;
731 switch (instrValue.kind) {
732 case "BinaryExpression": {
681 - valueKind = ValueKind.Immutable;
682 - effectKind = Effect.Read;
733 + valueKind = {
734 + kind: ValueKind.Immutable,
735 + reason: new Set([ValueReason.Other]),
736 + };
737 + effect = {
738 + kind: Effect.Read,
739 + reason: ValueReason.Other,
740 + };
741 break;
742 }
743 case "ArrayExpression": {
744 valueKind = hasContextRefOperand(state, instrValue)
687 - ? ValueKind.Context
688 - : ValueKind.Mutable;
689 - effectKind = Effect.Capture;
745 + ? {
746 + kind: ValueKind.Context,
747 + reason: new Set([ValueReason.Other]),
748 + }
749 + : { kind: ValueKind.Mutable, reason: new Set([ValueReason.Other]) };
750 + effect = { kind: Effect.Capture, reason: ValueReason.Other };
751 lvalueEffect = Effect.Store;
752 break;
753 }
754 case "NewExpression": {
694 - valueKind = ValueKind.Mutable;
695 - effectKind = Effect.ConditionallyMutate;
755 + valueKind = {
756 + kind: ValueKind.Mutable,
757 + reason: new Set([ValueReason.Other]),
758 + };
759 + effect = {
760 + kind: Effect.ConditionallyMutate,
761 + reason: ValueReason.Other,
762 + };
763 break;
764 }
765 case "ObjectExpression": {
766 valueKind = hasContextRefOperand(state, instrValue)
700 - ? ValueKind.Context
701 - : ValueKind.Mutable;
767 + ? {
768 + kind: ValueKind.Context,
769 + reason: new Set([ValueReason.Other]),
770 + }
771 + : { kind: ValueKind.Mutable, reason: new Set([ValueReason.Other]) };
772
773 for (const property of instrValue.properties) {
774 switch (property.kind) {
775 case "ObjectProperty": {
776 if (property.key.kind === "computed") {
777 // Object keys must be primitives, so we know they're frozen at this point
708 - state.reference(property.key.name, Effect.Freeze);
778 + state.reference(
779 + property.key.name,
780 + Effect.Freeze,
781 + ValueReason.Other
782 + );
783 }
784 // Object construction captures but does not modify the key/property values
711 - state.reference(property.place, Effect.Capture);
785 + state.reference(
786 + property.place,
787 + Effect.Capture,
788 + ValueReason.Other
789 + );
790 break;
791 }
792 case "Spread": {
793 // Object construction captures but does not modify the key/property values
716 - state.reference(property.place, Effect.Capture);
794 + state.reference(
795 + property.place,
796 + Effect.Capture,
797 + ValueReason.Other
798 + );
799 break;
800 }
801 default: {
@@ -731,28 +813,49 @@ function inferBlock(
813 continue;
814 }
815 case "UnaryExpression": {
734 - valueKind = ValueKind.Immutable;
735 - effectKind = Effect.Read;
816 + valueKind = {
817 + kind: ValueKind.Immutable,
818 + reason: new Set([ValueReason.Other]),
819 + };
820 + effect = { kind: Effect.Read, reason: ValueReason.Other };
821 break;
822 }
823 case "UnsupportedNode": {
824 // TODO: handle other statement kinds
740 - valueKind = ValueKind.Mutable;
825 + valueKind = {
826 + kind: ValueKind.Mutable,
827 + reason: new Set([ValueReason.Other]),
828 + };
829 break;
830 }
831 case "JsxExpression": {
744 - valueKind = ValueKind.Frozen;
745 - effectKind = Effect.Freeze;
832 + valueKind = {
833 + kind: ValueKind.Frozen,
834 + reason: new Set([ValueReason.Other]),
835 + };
836 + effect = { kind: Effect.Freeze, reason: ValueReason.JsxCaptured };
837 break;
838 }
839 case "JsxFragment": {
749 - valueKind = ValueKind.Frozen;
750 - effectKind = Effect.Freeze;
840 + valueKind = {
841 + kind: ValueKind.Frozen,
842 + reason: new Set([ValueReason.Other]),
843 + };
844 + effect = {
845 + kind: Effect.Freeze,
846 + reason: ValueReason.Other,
847 + };
848 break;
849 }
850 case "TaggedTemplateExpression": {
754 - valueKind = ValueKind.Mutable;
755 - effectKind = Effect.ConditionallyMutate;
851 + valueKind = {
852 + kind: ValueKind.Mutable,
853 + reason: new Set([ValueReason.Other]),
854 + };
855 + effect = {
856 + kind: Effect.ConditionallyMutate,
857 + reason: ValueReason.Other,
858 + };
859 break;
860 }
861 case "TemplateLiteral": {
@@ -760,21 +863,38 @@ function inferBlock(
863 * template literal (with no tag function) always produces
864 * an immutable string
865 */
763 - valueKind = ValueKind.Immutable;
764 - effectKind = Effect.Read;
866 + valueKind = {
867 + kind: ValueKind.Immutable,
868 + reason: new Set([ValueReason.Other]),
869 + };
870 + effect = { kind: Effect.Read, reason: ValueReason.Other };
871 break;
872 }
873 case "RegExpLiteral": {
874 // RegExp instances are mutable objects
769 - valueKind = ValueKind.Mutable;
770 - effectKind = Effect.ConditionallyMutate;
875 + valueKind = {
876 + kind: ValueKind.Mutable,
877 + reason: new Set([ValueReason.Other]),
878 + };
879 + effect = {
880 + kind: Effect.ConditionallyMutate,
881 + reason: ValueReason.Other,
882 + };
883 break;
884 }
773 - case "Debugger":
885 case "LoadGlobal":
886 + valueKind = {
887 + kind: ValueKind.Immutable,
888 + reason: new Set([ValueReason.Global]),
889 + };
890 + break;
891 + case "Debugger":
892 case "JSXText":
893 case "Primitive": {
777 - valueKind = ValueKind.Immutable;
894 + valueKind = {
895 + kind: ValueKind.Immutable,
896 + reason: new Set([ValueReason.Other]),
897 + };
898 break;
899 }
900 case "ObjectMethod":
@@ -783,7 +903,8 @@ function inferBlock(
903 for (const operand of eachInstructionOperand(instr)) {
904 state.reference(
905 operand,
786 - operand.effect === Effect.Unknown ? Effect.Read : operand.effect
906 + operand.effect === Effect.Unknown ? Effect.Read : operand.effect,
907 + ValueReason.Other
908 );
909 hasMutableOperand ||= isMutableEffect(operand.effect, operand.loc);
910 }
@@ -791,10 +912,10 @@ function inferBlock(
912 * If a closure did not capture any mutable values, then we can consider it to be
913 * frozen, which allows it to be independently memoized.
914 */
794 - state.initialize(
795 - instrValue,
796 - hasMutableOperand ? ValueKind.Mutable : ValueKind.Frozen
797 - );
915 + state.initialize(instrValue, {
916 + kind: hasMutableOperand ? ValueKind.Mutable : ValueKind.Frozen,
917 + reason: new Set([ValueReason.Other]),
918 + });
919 state.define(instr.lvalue, instrValue);
920 instr.lvalue.effect = Effect.Store;
921 continue;
@@ -807,23 +928,40 @@ function inferBlock(
928
929 const effects =
930 signature !== null ? getFunctionEffects(instrValue, signature) : null;
810 - const returnValueKind =
811 - signature !== null ? signature.returnValueKind : ValueKind.Mutable;
931 + const returnValueKind: AbstractValue =
932 + signature !== null
933 + ? {
934 + kind: signature.returnValueKind,
935 + reason: new Set([ValueReason.KnownReturnSignature]),
936 + }
937 + : { kind: ValueKind.Mutable, reason: new Set([ValueReason.Other]) };
938 let hasCaptureArgument = false;
939 for (let i = 0; i < instrValue.args.length; i++) {
940 const arg = instrValue.args[i];
941 const place = arg.kind === "Identifier" ? arg : arg.place;
942 if (effects !== null) {
817 - state.reference(place, effects[i]);
943 + state.reference(place, effects[i], ValueReason.Other);
944 } else {
819 - state.reference(place, Effect.ConditionallyMutate);
945 + state.reference(
946 + place,
947 + Effect.ConditionallyMutate,
948 + ValueReason.Other
949 + );
950 }
951 hasCaptureArgument ||= place.effect === Effect.Capture;
952 }
953 if (signature !== null) {
824 - state.reference(instrValue.callee, signature.calleeEffect);
954 + state.reference(
955 + instrValue.callee,
956 + signature.calleeEffect,
957 + ValueReason.Other
958 + );
959 } else {
826 - state.reference(instrValue.callee, Effect.ConditionallyMutate);
960 + state.reference(
961 + instrValue.callee,
962 + Effect.ConditionallyMutate,
963 + ValueReason.Other
964 + );
965 }
966 hasCaptureArgument ||= instrValue.callee.effect === Effect.Capture;
967
@@ -842,13 +980,21 @@ function inferBlock(
980 loc: instrValue.loc,
981 suggestions: null,
982 });
845 - state.reference(instrValue.property, Effect.Read);
983 + state.reference(instrValue.property, Effect.Read, ValueReason.Other);
984
985 const signature = getFunctionCallSignature(
986 env,
987 instrValue.property.identifier.type
988 );
989
990 + const returnValueKind: AbstractValue =
991 + signature !== null
992 + ? {
993 + kind: signature.returnValueKind,
994 + reason: new Set([ValueReason.Other]),
995 + }
996 + : { kind: ValueKind.Mutable, reason: new Set([ValueReason.Other]) };
997 +
998 if (
999 signature !== null &&
1000 signature.mutableOnlyIfOperandsAreMutable &&
@@ -860,10 +1006,14 @@ function inferBlock(
1006 */
1007 for (const arg of instrValue.args) {
1008 const place = arg.kind === "Identifier" ? arg : arg.place;
863 - state.reference(place, Effect.Read);
1009 + state.reference(place, Effect.Read, ValueReason.Other);
1010 }
865 - state.reference(instrValue.receiver, Effect.Capture);
866 - state.initialize(instrValue, signature.returnValueKind);
1011 + state.reference(
1012 + instrValue.receiver,
1013 + Effect.Capture,
1014 + ValueReason.Other
1015 + );
1016 + state.initialize(instrValue, returnValueKind);
1017 state.define(instr.lvalue, instrValue);
1018 instr.lvalue.effect =
1019 instrValue.receiver.effect === Effect.Capture
@@ -874,8 +1024,6 @@ function inferBlock(
1024
1025 const effects =
1026 signature !== null ? getFunctionEffects(instrValue, signature) : null;
877 - const returnValueKind =
878 - signature !== null ? signature.returnValueKind : ValueKind.Mutable;
1027 let hasCaptureArgument = false;
1028 for (let i = 0; i < instrValue.args.length; i++) {
1029 const arg = instrValue.args[i];
@@ -885,16 +1033,28 @@ function inferBlock(
1033 * If effects are inferred for an argument, we should fail invalid
1034 * mutating effects
1035 */
888 - state.reference(place, effects[i]);
1036 + state.reference(place, effects[i], ValueReason.Other);
1037 } else {
890 - state.reference(place, Effect.ConditionallyMutate);
1038 + state.reference(
1039 + place,
1040 + Effect.ConditionallyMutate,
1041 + ValueReason.Other
1042 + );
1043 }
1044 hasCaptureArgument ||= place.effect === Effect.Capture;
1045 }
1046 if (signature !== null) {
895 - state.reference(instrValue.receiver, signature.calleeEffect);
1047 + state.reference(
1048 + instrValue.receiver,
1049 + signature.calleeEffect,
1050 + ValueReason.Other
1051 + );
1052 } else {
897 - state.reference(instrValue.receiver, Effect.ConditionallyMutate);
1053 + state.reference(
1054 + instrValue.receiver,
1055 + Effect.ConditionallyMutate,
1056 + ValueReason.Other
1057 + );
1058 }
1059 hasCaptureArgument ||= instrValue.receiver.effect === Effect.Capture;
1060
@@ -907,11 +1067,11 @@ function inferBlock(
1067 }
1068 case "PropertyStore": {
1069 const effect =
910 - state.kind(instrValue.object) === ValueKind.Context
1070 + state.kind(instrValue.object).kind === ValueKind.Context
1071 ? Effect.ConditionallyMutate
1072 : Effect.Capture;
913 - state.reference(instrValue.value, effect);
914 - state.reference(instrValue.object, Effect.Store);
1073 + state.reference(instrValue.value, effect, ValueReason.Other);
1074 + state.reference(instrValue.object, Effect.Store, ValueReason.Other);
1075
1076 const lvalue = instr.lvalue;
1077 state.alias(lvalue, instrValue.value);
@@ -920,12 +1080,15 @@ function inferBlock(
1080 }
1081 case "PropertyDelete": {
1082 // `delete` returns a boolean (immutable) and modifies the object
923 - valueKind = ValueKind.Immutable;
924 - effectKind = Effect.Mutate;
1083 + valueKind = {
1084 + kind: ValueKind.Immutable,
1085 + reason: new Set([ValueReason.Other]),
1086 + };
1087 + effect = { kind: Effect.Mutate, reason: ValueReason.Other };
1088 break;
1089 }
1090 case "PropertyLoad": {
928 - state.reference(instrValue.object, Effect.Read);
1091 + state.reference(instrValue.object, Effect.Read, ValueReason.Other);
1092 const lvalue = instr.lvalue;
1093 lvalue.effect = Effect.ConditionallyMutate;
1094 state.initialize(instrValue, state.kind(instrValue.object));
@@ -934,12 +1097,12 @@ function inferBlock(
1097 }
1098 case "ComputedStore": {
1099 const effect =
937 - state.kind(instrValue.object) === ValueKind.Context
1100 + state.kind(instrValue.object).kind === ValueKind.Context
1101 ? Effect.ConditionallyMutate
1102 : Effect.Capture;
940 - state.reference(instrValue.value, effect);
941 - state.reference(instrValue.property, Effect.Capture);
942 - state.reference(instrValue.object, Effect.Store);
1103 + state.reference(instrValue.value, effect, ValueReason.Other);
1104 + state.reference(instrValue.property, Effect.Capture, ValueReason.Other);
1105 + state.reference(instrValue.object, Effect.Store, ValueReason.Other);
1106
1107 const lvalue = instr.lvalue;
1108 state.alias(lvalue, instrValue.value);
@@ -947,16 +1110,19 @@ function inferBlock(
1110 continue;
1111 }
1112 case "ComputedDelete": {
950 - state.reference(instrValue.object, Effect.Mutate);
951 - state.reference(instrValue.property, Effect.Read);
952 - state.initialize(instrValue, ValueKind.Immutable);
1113 + state.reference(instrValue.object, Effect.Mutate, ValueReason.Other);
1114 + state.reference(instrValue.property, Effect.Read, ValueReason.Other);
1115 + state.initialize(instrValue, {
1116 + kind: ValueKind.Immutable,
1117 + reason: new Set([ValueReason.Other]),
1118 + });
1119 state.define(instr.lvalue, instrValue);
1120 instr.lvalue.effect = Effect.Mutate;
1121 continue;
1122 }
1123 case "ComputedLoad": {
958 - state.reference(instrValue.object, Effect.Read);
959 - state.reference(instrValue.property, Effect.Read);
1124 + state.reference(instrValue.object, Effect.Read, ValueReason.Other);
1125 + state.reference(instrValue.property, Effect.Read, ValueReason.Other);
1126 const lvalue = instr.lvalue;
1127 lvalue.effect = Effect.ConditionallyMutate;
1128 state.initialize(instrValue, state.kind(instrValue.object));
@@ -970,7 +1136,11 @@ function inferBlock(
1136 * It also means that any side-effects which would occur as part of the promise evaluation
1137 * will occur.
1138 */
973 - state.reference(instrValue.value, Effect.ConditionallyMutate);
1139 + state.reference(
1140 + instrValue.value,
1141 + Effect.ConditionallyMutate,
1142 + ValueReason.Other
1143 + );
1144 const lvalue = instr.lvalue;
1145 lvalue.effect = Effect.ConditionallyMutate;
1146 state.alias(lvalue, instrValue.value);
@@ -986,7 +1156,7 @@ function inferBlock(
1156 * ```
1157 */
1158 state.initialize(instrValue, state.kind(instrValue.value));
989 - state.reference(instrValue.value, Effect.Read);
1159 + state.reference(instrValue.value, Effect.Read, ValueReason.Other);
1160 const lvalue = instr.lvalue;
1161 lvalue.effect = Effect.ConditionallyMutate;
1162 state.alias(lvalue, instrValue.value);
@@ -995,22 +1165,24 @@ function inferBlock(
1165 case "LoadLocal": {
1166 const lvalue = instr.lvalue;
1167 const effect =
998 - state.isDefined(lvalue) && state.kind(lvalue) === ValueKind.Context
1168 + state.isDefined(lvalue) &&
1169 + state.kind(lvalue).kind === ValueKind.Context
1170 ? Effect.ConditionallyMutate
1171 : Effect.Capture;
1001 - state.reference(instrValue.place, effect);
1172 + state.reference(instrValue.place, effect, ValueReason.Other);
1173 lvalue.effect = Effect.ConditionallyMutate;
1174 // direct aliasing: `a = b`;
1175 state.alias(lvalue, instrValue.place);
1176 continue;
1177 }
1178 case "LoadContext": {
1008 - state.reference(instrValue.place, Effect.Capture);
1179 + state.reference(instrValue.place, Effect.Capture, ValueReason.Other);
1180 const lvalue = instr.lvalue;
1181 lvalue.effect = Effect.ConditionallyMutate;
1182 const valueKind = state.kind(instrValue.place);
1183 CompilerError.invariant(
1013 - valueKind === ValueKind.Mutable || valueKind === ValueKind.Context,
1184 + valueKind.kind === ValueKind.Mutable ||
1185 + valueKind.kind === ValueKind.Context,
1186 {
1187 reason:
1188 "[InferReferenceEffects] Context variables are always mutable.",
@@ -1029,14 +1201,23 @@ function inferBlock(
1201 value,
1202 // Catch params may be aliased to mutable values
1203 instrValue.lvalue.kind === InstructionKind.Catch
1032 - ? ValueKind.Mutable
1033 - : ValueKind.Immutable
1204 + ? {
1205 + kind: ValueKind.Mutable,
1206 + reason: new Set([ValueReason.Other]),
1207 + }
1208 + : {
1209 + kind: ValueKind.Immutable,
1210 + reason: new Set([ValueReason.Other]),
1211 + }
1212 );
1213 state.define(instrValue.lvalue.place, value);
1214 continue;
1215 }
1216 case "DeclareContext": {
1039 - state.initialize(instrValue, ValueKind.Mutable);
1217 + state.initialize(instrValue, {
1218 + kind: ValueKind.Mutable,
1219 + reason: new Set([ValueReason.Other]),
1220 + });
1221 state.define(instrValue.lvalue.place, instrValue);
1222 continue;
1223 }
@@ -1044,10 +1225,10 @@ function inferBlock(
1225 case "PrefixUpdate": {
1226 const effect =
1227 state.isDefined(instrValue.lvalue) &&
1047 - state.kind(instrValue.lvalue) === ValueKind.Context
1228 + state.kind(instrValue.lvalue).kind === ValueKind.Context
1229 ? Effect.ConditionallyMutate
1230 : Effect.Capture;
1050 - state.reference(instrValue.value, effect);
1231 + state.reference(instrValue.value, effect, ValueReason.Other);
1232
1233 const lvalue = instr.lvalue;
1234 state.alias(lvalue, instrValue.value);
@@ -1065,10 +1246,10 @@ function inferBlock(
1246 case "StoreLocal": {
1247 const effect =
1248 state.isDefined(instrValue.lvalue.place) &&
1068 - state.kind(instrValue.lvalue.place) === ValueKind.Context
1249 + state.kind(instrValue.lvalue.place).kind === ValueKind.Context
1250 ? Effect.ConditionallyMutate
1251 : Effect.Capture;
1071 - state.reference(instrValue.value, effect);
1252 + state.reference(instrValue.value, effect, ValueReason.Other);
1253
1254 const lvalue = instr.lvalue;
1255 state.alias(lvalue, instrValue.value);
@@ -1084,8 +1265,16 @@ function inferBlock(
1265 continue;
1266 }
1267 case "StoreContext": {
1087 - state.reference(instrValue.value, Effect.ConditionallyMutate);
1088 - state.reference(instrValue.lvalue.place, Effect.Mutate);
1268 + state.reference(
1269 + instrValue.value,
1270 + Effect.ConditionallyMutate,
1271 + ValueReason.Other
1272 + );
1273 + state.reference(
1274 + instrValue.lvalue.place,
1275 + Effect.Mutate,
1276 + ValueReason.Other
1277 + );
1278
1279 const lvalue = instr.lvalue;
1280 state.alias(lvalue, instrValue.value);
@@ -1097,13 +1286,13 @@ function inferBlock(
1286 for (const place of eachPatternOperand(instrValue.lvalue.pattern)) {
1287 if (
1288 state.isDefined(place) &&
1100 - state.kind(place) === ValueKind.Context
1289 + state.kind(place).kind === ValueKind.Context
1290 ) {
1291 effect = Effect.ConditionallyMutate;
1292 break;
1293 }
1294 }
1106 - state.reference(instrValue.value, effect);
1295 + state.reference(instrValue.value, effect, ValueReason.Other);
1296
1297 const lvalue = instr.lvalue;
1298 state.alias(lvalue, instrValue.value);
@@ -1121,15 +1310,21 @@ function inferBlock(
1310 continue;
1311 }
1312 case "NextIterableOf": {
1124 - effectKind = Effect.Capture;
1313 + effect = { kind: Effect.Capture, reason: ValueReason.Other };
1314 lvalueEffect = Effect.Store;
1126 - valueKind = ValueKind.Mutable;
1315 + valueKind = {
1316 + kind: ValueKind.Mutable,
1317 + reason: new Set([ValueReason.Other]),
1318 + };
1319 break;
1320 }
1321 case "NextPropertyOf": {
1130 - effectKind = Effect.Read;
1322 + effect = { kind: Effect.Read, reason: ValueReason.Other };
1323 lvalueEffect = Effect.Store;
1132 - valueKind = ValueKind.Immutable;
1324 + valueKind = {
1325 + kind: ValueKind.Immutable,
1326 + reason: new Set([ValueReason.Other]),
1327 + };
1328 break;
1329 }
1330 default: {
@@ -1138,13 +1333,13 @@ function inferBlock(
1333 }
1334
1335 for (const operand of eachInstructionOperand(instr)) {
1141 - CompilerError.invariant(effectKind != null, {
1336 + CompilerError.invariant(effect != null, {
1337 reason: `effectKind must be set for instruction value \`${instrValue.kind}\``,
1338 description: null,
1339 loc: instrValue.loc,
1340 suggestions: null,
1341 });
1147 - state.reference(operand, effectKind);
1342 + state.reference(operand, effect.kind, effect.reason);
1343 }
1344
1345 state.initialize(instrValue, valueKind);
@@ -1157,7 +1352,7 @@ function inferBlock(
1352 if (block.terminal.kind === "return" || block.terminal.kind === "throw") {
1353 if (
1354 state.isDefined(operand) &&
1160 - state.kind(operand) === ValueKind.Context
1355 + state.kind(operand).kind === ValueKind.Context
1356 ) {
1357 effect = Effect.ConditionallyMutate;
1358 } else {
@@ -1166,7 +1361,7 @@ function inferBlock(
1361 } else {
1362 effect = Effect.Read;
1363 }
1169 - state.reference(operand, effect);
1364 + state.reference(operand, effect, ValueReason.Other);
1365 }
1366 }
1367
@@ -1175,7 +1370,10 @@ function hasContextRefOperand(
1370 instrValue: InstructionValue
1371 ): boolean {
1372 for (const place of eachInstructionValueOperand(instrValue)) {
1178 - if (state.isDefined(place) && state.kind(place) === ValueKind.Context) {
1373 + if (
1374 + state.isDefined(place) &&
1375 + state.kind(place).kind === ValueKind.Context
1376 + ) {
1377 return true;
1378 }
1379 }
@@ -1242,7 +1440,7 @@ function areArgumentsImmutableAndNonMutating(
1440 ): boolean {
1441 for (const arg of args) {
1442 const place = arg.kind === "Identifier" ? arg : arg.place;
1245 - const kind = state.kind(place);
1443 + const kind = state.kind(place).kind;
1444 switch (kind) {
1445 case ValueKind.Immutable:
1446 case ValueKind.Frozen: {
@@ -1274,3 +1472,15 @@ function areArgumentsImmutableAndNonMutating(
1472 }
1473 return true;
1474 }
1475 +
1476 +function getWriteErrorReason(abstractValue: AbstractValue): string {
1477 + if (abstractValue.reason.has(ValueReason.Global)) {
1478 + return "Writing to a variable defined outside a component or hook is not allowed. Consider using an effect.";
1479 + } else if (abstractValue.reason.has(ValueReason.JsxCaptured)) {
1480 + return "Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX.";
1481 + } else if (abstractValue.reason.has(ValueReason.KnownReturnSignature)) {
1482 + return "Mutating a value returned from a function that should not be mutated.";
1483 + } else {
1484 + return "This mutates a global or a variable after it was passed to React, which means that React cannot observe changes to it.";
1485 + }
1486 +}
compiler/packages/babel-plugin-react-forget/src/__tests__/envConfig-test.ts
+2 -6
@@ -5,12 +5,8 @@
5 * LICENSE file in the root directory of this source tree.
6 */
7
8 -import {
9 - CompilerError,
10 - Effect,
11 - ValueKind,
12 - validateEnvironmentConfig,
13 -} from "..";
8 +import { Effect, validateEnvironmentConfig } from "..";
9 +import { ValueKind } from "../HIR";
10
11 describe("parseConfigPragma()", () => {
12 it("passing null throws", () => {
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-array-push-frozen.expect.md
+1 -1
@@ -15,7 +15,7 @@ function Component(props) {
15 ## Error
16
17 ```
18 -[ReactForget] InvalidReact: This mutates a global or a variable after it was passed to React, which means that React cannot observe changes to it (4:4)
18 +[ReactForget] InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX. (4:4)
19 ```
20
21
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-computed-store-to-frozen-value.expect.md
+1 -1
@@ -16,7 +16,7 @@ function Component(props) {
16 ## Error
17
18 ```
19 -[ReactForget] InvalidReact: This mutates a global or a variable after it was passed to React, which means that React cannot observe changes to it (5:5)
19 +[ReactForget] InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX. (5:5)
20 ```
21
22
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-delete-computed-property-of-frozen-value.expect.md
+1 -1
@@ -16,7 +16,7 @@ function Component(props) {
16 ## Error
17
18 ```
19 -[ReactForget] InvalidReact: This mutates a global or a variable after it was passed to React, which means that React cannot observe changes to it (5:5)
19 +[ReactForget] InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX. (5:5)
20 ```
21
22
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-delete-property-of-frozen-value.expect.md
+1 -1
@@ -16,7 +16,7 @@ function Component(props) {
16 ## Error
17
18 ```
19 -[ReactForget] InvalidReact: This mutates a global or a variable after it was passed to React, which means that React cannot observe changes to it (5:5)
19 +[ReactForget] InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX. (5:5)
20 ```
21
22
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-function-expression-mutates-immutable-value.expect.md
+1 -1
@@ -18,7 +18,7 @@ function Component(props) {
18 ## Error
19
20 ```
21 -[ReactForget] InvalidReact: This mutates a global or a variable after it was passed to React, which means that React cannot observe changes to it (5:5)
21 +[ReactForget] InvalidReact: Mutating a value returned from a function that should not be mutated. (5:5)
22 ```
23
24
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutate-after-aliased-freeze.expect.md
+1 -1
@@ -25,7 +25,7 @@ function Component(props) {
25 ## Error
26
27 ```
28 -[ReactForget] InvalidReact: This mutates a global or a variable after it was passed to React, which means that React cannot observe changes to it (13:13)
28 +[ReactForget] InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX. (13:13)
29 ```
30
31
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-mutate-after-freeze.expect.md
+1 -1
@@ -19,7 +19,7 @@ function Component(props) {
19 ## Error
20
21 ```
22 -[ReactForget] InvalidReact: This mutates a global or a variable after it was passed to React, which means that React cannot observe changes to it (7:7)
22 +[ReactForget] InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX. (7:7)
23 ```
24
25
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.invalid-property-store-to-frozen-value.expect.md
+1 -1
@@ -16,7 +16,7 @@ function Component(props) {
16 ## Error
17
18 ```
19 -[ReactForget] InvalidReact: This mutates a global or a variable after it was passed to React, which means that React cannot observe changes to it (5:5)
19 +[ReactForget] InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX. (5:5)
20 ```
21
22
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.mutate-property-from-global.expect.md
+1 -1
@@ -15,7 +15,7 @@ function Foo() {
15 ## Error
16
17 ```
18 -[ReactForget] InvalidReact: This mutates a global or a variable after it was passed to React, which means that React cannot observe changes to it (4:4)
18 +[ReactForget] InvalidReact: Writing to a variable defined outside a component or hook is not allowed. Consider using an effect. (4:4)
19 ```
20
21
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.store-property-in-global.expect.md
+1 -1
@@ -15,7 +15,7 @@ function Foo() {
15 ## Error
16
17 ```
18 -[ReactForget] InvalidReact: This mutates a global or a variable after it was passed to React, which means that React cannot observe changes to it (4:4)
18 +[ReactForget] InvalidReact: Writing to a variable defined outside a component or hook is not allowed. Consider using an effect. (4:4)
19 ```
20
21
\ No newline at end of file