Compare commits

..

3 Commits

Author SHA1 Message Date
Joe Savona
2145ea7b59 [compiler] Remove now-unused FunctionEffect type
The new mutation/aliasing model significantly expands on the idea of FunctionEffect. The type (and its usage in HIRFunction.effects) was only necessary for the now-deleted old inference model so we can clean up this code now.
2025-08-15 15:23:06 -07:00
Joseph Savona
eaf6adb127 [compiler][wip] Remove old mutation/aliasing implementation (#34028)
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34028).
* #34029
* __->__ #34028
2025-08-15 15:21:28 -07:00
Joseph Savona
6ffcac8558 [compiler] Add support for diagnostic hints (#34126)
Hints are meant as additional information to present to the developer
about an error. The first use-case here is for the suggestion to name
refs with "-Ref" if we encounter a mutation that looks like it might be
a ref. The original error printing used a second error detail which
printed the source code twice, a hint with just extra text is less
noisy.
2025-08-15 15:09:27 -07:00
8 changed files with 35 additions and 82 deletions

View File

@@ -58,11 +58,15 @@ export type CompilerDiagnosticDetail =
/**
* A/the source of the error
*/
{
kind: 'error';
loc: SourceLocation | null;
message: string;
};
| {
kind: 'error';
loc: SourceLocation | null;
message: string;
}
| {
kind: 'hint';
message: string;
};
export enum CompilerSuggestionOperation {
InsertBefore,
@@ -134,7 +138,12 @@ export class CompilerDiagnostic {
}
primaryLocation(): SourceLocation | null {
return this.options.details.filter(d => d.kind === 'error')[0]?.loc ?? null;
const firstErrorDetail = this.options.details.filter(
d => d.kind === 'error',
)[0];
return firstErrorDetail != null && firstErrorDetail.kind === 'error'
? firstErrorDetail.loc
: null;
}
printErrorMessage(source: string, options: PrintErrorMessageOptions): string {
@@ -167,9 +176,14 @@ export class CompilerDiagnostic {
buffer.push(codeFrame);
break;
}
case 'hint': {
buffer.push('\n\n');
buffer.push(detail.message);
break;
}
default: {
assertExhaustive(
detail.kind,
detail,
`Unexpected detail kind ${(detail as any).kind}`,
);
}

View File

@@ -7,7 +7,7 @@
import {BindingKind} from '@babel/traverse';
import * as t from '@babel/types';
import {CompilerError, CompilerErrorDetailOptions} from '../CompilerError';
import {CompilerError} from '../CompilerError';
import {assertExhaustive} from '../Utils/utils';
import {Environment, ReactFunctionType} from './Environment';
import type {HookKind} from './ObjectShape';
@@ -282,30 +282,13 @@ export type HIRFunction = {
returnTypeAnnotation: t.FlowType | t.TSType | null;
returns: Place;
context: Array<Place>;
effects: Array<FunctionEffect> | null;
body: HIR;
generator: boolean;
async: boolean;
directives: Array<string>;
aliasingEffects?: Array<AliasingEffect> | null;
aliasingEffects: Array<AliasingEffect> | null;
};
export type FunctionEffect =
| {
kind: 'GlobalMutation';
error: CompilerErrorDetailOptions;
}
| {
kind: 'ReactMutation';
error: CompilerErrorDetailOptions;
}
| {
kind: 'ContextMutation';
places: ReadonlySet<Place>;
effect: Effect;
loc: SourceLocation;
};
/*
* Each reactive scope may have its own control-flow, so the instructions form
* a control-flow graph. The graph comprises a set of basic blocks which reference

View File

@@ -554,23 +554,11 @@ export function printInstructionValue(instrValue: ReactiveValue): string {
const context = instrValue.loweredFunc.func.context
.map(dep => printPlace(dep))
.join(',');
const effects =
instrValue.loweredFunc.func.effects
?.map(effect => {
if (effect.kind === 'ContextMutation') {
return `ContextMutation places=[${[...effect.places]
.map(place => printPlace(place))
.join(', ')}] effect=${effect.effect}`;
} else {
return `GlobalMutation`;
}
})
.join(', ') ?? '';
const aliasingEffects =
instrValue.loweredFunc.func.aliasingEffects
?.map(printAliasingEffect)
?.join(', ') ?? '';
value = `${kind} ${name} @context[${context}] @effects[${effects}] @aliasingEffects=[${aliasingEffects}]\n${fn}`;
value = `${kind} ${name} @context[${context}] @aliasingEffects=[${aliasingEffects}]\n${fn}`;
break;
}
case 'TaggedTemplateExpression': {

View File

@@ -465,8 +465,7 @@ function applySignature(
effect.reason?.kind === 'AssignCurrentProperty'
) {
diagnostic.withDetail({
kind: 'error',
loc: effect.value.loc,
kind: 'hint',
message: `Hint: If this value is a Ref (value returned by \`useRef()\`), rename the variable to end in "Ref".`,
});
}
@@ -1089,8 +1088,7 @@ function applyEffect(
effect.reason?.kind === 'AssignCurrentProperty'
) {
diagnostic.withDetail({
kind: 'error',
loc: effect.value.loc,
kind: 'hint',
message: `Hint: If this value is a Ref (value returned by \`useRef()\`), rename the variable to end in "Ref".`,
});
}

View File

@@ -255,7 +255,6 @@ function emitSelectorFn(env: Environment, keys: Array<string>): Instruction {
returnTypeAnnotation: null,
returns: createTemporaryPlace(env, GeneratedSource),
context: [],
effects: null,
body: {
entry: block.id,
blocks: new Map([[block.id, block]]),
@@ -263,6 +262,7 @@ function emitSelectorFn(env: Environment, keys: Array<string>): Instruction {
generator: false,
async: false,
directives: [],
aliasingEffects: [],
};
reversePostorderBlocks(fn.body);

View File

@@ -370,7 +370,6 @@ function emitOutlinedFn(
returnTypeAnnotation: null,
returns: createTemporaryPlace(env, GeneratedSource),
context: [],
effects: null,
body: {
entry: block.id,
blocks: new Map([[block.id, block]]),
@@ -378,6 +377,7 @@ function emitOutlinedFn(
generator: false,
async: false,
directives: [],
aliasingEffects: [],
};
return fn;
}

View File

@@ -7,10 +7,8 @@
import {CompilerDiagnostic, CompilerError, Effect, ErrorSeverity} from '..';
import {
FunctionEffect,
HIRFunction,
IdentifierId,
isMutableEffect,
isRefOrRefLikeMutableType,
Place,
} from '../HIR';
@@ -18,8 +16,8 @@ import {
eachInstructionValueOperand,
eachTerminalOperand,
} from '../HIR/visitors';
import {AliasingEffect} from '../Inference/AliasingEffects';
import {Result} from '../Utils/Result';
import {Iterable_some} from '../Utils/utils';
/**
* Validates that functions with known mutations (ie due to types) cannot be passed
@@ -50,14 +48,14 @@ export function validateNoFreezingKnownMutableFunctions(
const errors = new CompilerError();
const contextMutationEffects: Map<
IdentifierId,
Extract<FunctionEffect, {kind: 'ContextMutation'}>
Extract<AliasingEffect, {kind: 'Mutate'} | {kind: 'MutateTransitive'}>
> = new Map();
function visitOperand(operand: Place): void {
if (operand.effect === Effect.Freeze) {
const effect = contextMutationEffects.get(operand.identifier.id);
if (effect != null) {
const place = [...effect.places][0];
const place = effect.value;
const variable =
place != null &&
place.identifier.name != null &&
@@ -77,7 +75,7 @@ export function validateNoFreezingKnownMutableFunctions(
})
.withDetail({
kind: 'error',
loc: effect.loc,
loc: effect.value.loc,
message: `This modifies ${variable}`,
}),
);
@@ -108,24 +106,7 @@ export function validateNoFreezingKnownMutableFunctions(
break;
}
case 'FunctionExpression': {
const knownMutation = (value.loweredFunc.func.effects ?? []).find(
effect => {
return (
effect.kind === 'ContextMutation' &&
(effect.effect === Effect.Store ||
effect.effect === Effect.Mutate) &&
Iterable_some(effect.places, place => {
return (
isMutableEffect(place.effect, place.loc) &&
!isRefOrRefLikeMutableType(place.identifier.type)
);
})
);
},
);
if (knownMutation && knownMutation.kind === 'ContextMutation') {
contextMutationEffects.set(lvalue.identifier.id, knownMutation);
} else if (value.loweredFunc.func.aliasingEffects != null) {
if (value.loweredFunc.func.aliasingEffects != null) {
const context = new Set(
value.loweredFunc.func.context.map(p => p.identifier.id),
);
@@ -146,12 +127,7 @@ export function validateNoFreezingKnownMutableFunctions(
context.has(effect.value.identifier.id) &&
!isRefOrRefLikeMutableType(effect.value.identifier.type)
) {
contextMutationEffects.set(lvalue.identifier.id, {
kind: 'ContextMutation',
effect: Effect.Mutate,
loc: effect.value.loc,
places: new Set([effect.value]),
});
contextMutationEffects.set(lvalue.identifier.id, effect);
break effects;
}
break;

View File

@@ -30,13 +30,7 @@ Modifying a value returned from a hook is not allowed. Consider moving the modif
7 | }
8 |
3 | component Foo() {
4 | const foo = useFoo();
> 5 | foo.current = true;
| ^^^ Hint: If this value is a Ref (value returned by `useRef()`), rename the variable to end in "Ref".
6 | return <div />;
7 | }
8 |
Hint: If this value is a Ref (value returned by `useRef()`), rename the variable to end in "Ref".
```