Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,15 @@ function runWithEnvironment(
outlineJSX(hir);
}

if (env.config.enableNameAnonymousFunctions) {
nameAnonymousFunctions(hir);
log({
kind: 'hir',
name: 'NameAnonymousFunctions',
value: hir,
});
}

if (env.config.enableFunctionOutlining) {
outlineFunctions(hir, fbtOperands);
log({kind: 'hir', name: 'OutlineFunctions', value: hir});
Expand Down Expand Up @@ -415,15 +424,6 @@ function runWithEnvironment(
});
}

if (env.config.enableNameAnonymousFunctions) {
nameAnonymousFunctions(hir);
log({
kind: 'hir',
name: 'NameAnonymougFunctions',
value: hir,
});
}

const reactiveFunction = buildReactiveFunction(hir);
log({
kind: 'reactive',
Expand Down
23 changes: 15 additions & 8 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import {
makePropertyLiteral,
makeType,
promoteTemporary,
validateIdentifierName,
} from './HIR';
import HIRBuilder, {Bindings, createTemporaryPlace} from './HIRBuilder';
import {BuiltInArrayId} from './ObjectShape';
Expand Down Expand Up @@ -213,6 +214,16 @@ export function lower(
);
}

let validatedId: HIRFunction['id'] = null;
if (id != null) {
const idResult = validateIdentifierName(id);
if (idResult.isErr()) {
builder.errors.merge(idResult.unwrapErr());
} else {
validatedId = idResult.unwrap().value;
}
}

if (builder.errors.hasAnyErrors()) {
return Err(builder.errors);
}
Expand All @@ -234,7 +245,8 @@ export function lower(
);

return Ok({
id,
id: validatedId,
nameHint: null,
params,
fnType: bindings == null ? env.fnType : 'Other',
returnTypeAnnotation: null, // TODO: extract the actual return type node if present
Expand Down Expand Up @@ -3563,19 +3575,14 @@ function lowerFunctionToValue(
): InstructionValue {
const exprNode = expr.node;
const exprLoc = exprNode.loc ?? GeneratedSource;
let name: string | null = null;
if (expr.isFunctionExpression()) {
name = expr.get('id')?.node?.name ?? null;
} else if (expr.isFunctionDeclaration()) {
name = expr.get('id')?.node?.name ?? null;
}
const loweredFunc = lowerFunction(builder, expr);
if (!loweredFunc) {
return {kind: 'UnsupportedNode', node: exprNode, loc: exprLoc};
}
return {
kind: 'FunctionExpression',
name,
name: loweredFunc.func.id,
nameHint: null,
type: expr.node.type,
loc: exprLoc,
loweredFunc,
Expand Down
79 changes: 46 additions & 33 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@

import {BindingKind} from '@babel/traverse';
import * as t from '@babel/types';
import {CompilerError} from '../CompilerError';
import {
CompilerDiagnostic,
CompilerError,
ErrorCategory,
} from '../CompilerError';
import {assertExhaustive} from '../Utils/utils';
import {Environment, ReactFunctionType} from './Environment';
import type {HookKind} from './ObjectShape';
Expand Down Expand Up @@ -54,7 +58,8 @@ export type SourceLocation = t.SourceLocation | typeof GeneratedSource;
*/
export type ReactiveFunction = {
loc: SourceLocation;
id: string | null;
id: ValidIdentifierName | null;
nameHint: string | null;
params: Array<Place | SpreadPattern>;
generator: boolean;
async: boolean;
Expand Down Expand Up @@ -276,7 +281,8 @@ export type ReactiveTryTerminal = {
// A function lowered to HIR form, ie where its body is lowered to an HIR control-flow graph
export type HIRFunction = {
loc: SourceLocation;
id: string | null;
id: ValidIdentifierName | null;
nameHint: string | null;
fnType: ReactFunctionType;
env: Environment;
params: Array<Place | SpreadPattern>;
Expand Down Expand Up @@ -1124,7 +1130,8 @@ export type JsxAttribute =

export type FunctionExpression = {
kind: 'FunctionExpression';
name: string | null;
name: ValidIdentifierName | null;
nameHint: string | null;
loweredFunc: LoweredFunction;
type:
| 'ArrowFunctionExpression'
Expand Down Expand Up @@ -1301,11 +1308,41 @@ export function forkTemporaryIdentifier(

export function validateIdentifierName(
name: string,
): Result<ValidIdentifierName, null> {
if (isReservedWord(name) || !t.isValidIdentifier(name)) {
return Err(null);
): Result<ValidatedIdentifier, CompilerError> {
if (isReservedWord(name)) {
const error = new CompilerError();
error.pushDiagnostic(
CompilerDiagnostic.create({
category: ErrorCategory.Syntax,
reason: 'Expected a non-reserved identifier name',
description: `\`${name}\` is a reserved word in JavaScript and cannot be used as an identifier name`,
suggestions: null,
}).withDetails({
kind: 'error',
loc: GeneratedSource,
message: 'reserved word',
}),
);
return Err(error);
} else if (!t.isValidIdentifier(name)) {
const error = new CompilerError();
error.pushDiagnostic(
CompilerDiagnostic.create({
category: ErrorCategory.Syntax,
reason: `Expected a valid identifier name`,
description: `\`${name}\` is not a valid JavaScript identifier`,
suggestions: null,
}).withDetails({
kind: 'error',
loc: GeneratedSource,
message: 'reserved word',
}),
);
}
return Ok(makeIdentifierName(name).value);
return Ok({
kind: 'named',
value: name as ValidIdentifierName,
});
}

/**
Expand All @@ -1314,31 +1351,7 @@ export function validateIdentifierName(
* original source code.
*/
export function makeIdentifierName(name: string): ValidatedIdentifier {
if (isReservedWord(name)) {
CompilerError.throwInvalidJS({
reason: 'Expected a non-reserved identifier name',
loc: GeneratedSource,
description: `\`${name}\` is a reserved word in JavaScript and cannot be used as an identifier name`,
suggestions: null,
});
} else {
CompilerError.invariant(t.isValidIdentifier(name), {
reason: `Expected a valid identifier name`,
description: `\`${name}\` is not a valid JavaScript identifier`,
details: [
{
kind: 'error',
loc: GeneratedSource,
message: null,
},
],
suggestions: null,
});
}
return {
kind: 'named',
value: name as ValidIdentifierName,
};
return validateIdentifierName(name).unwrap();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ export function printFunction(fn: HIRFunction): string {
} else {
definition += '<<anonymous>>';
}
if (fn.nameHint != null) {
definition += ` ${fn.nameHint}`;
}
if (fn.params.length !== 0) {
definition +=
'(' +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -748,10 +748,14 @@ function applyEffect(
case 'Alias':
case 'Capture': {
CompilerError.invariant(
effect.kind === 'Capture' || initialized.has(effect.into.identifier.id),
effect.kind === 'Capture' ||
effect.kind === 'MaybeAlias' ||
initialized.has(effect.into.identifier.id),
{
reason: `Expected destination value to already be initialized within this instruction for Alias effect`,
description: `Destination ${printPlace(effect.into)} is not initialized in this instruction`,
reason: `Expected destination to already be initialized within this instruction`,
description:
`Destination ${printPlace(effect.into)} is not initialized in this ` +
`instruction for effect ${printAliasingEffect(effect)}`,
details: [
{
kind: 'error',
Expand All @@ -767,49 +771,67 @@ function applyEffect(
* copy-on-write semantics, then we can prune the effect
*/
const intoKind = state.kind(effect.into).kind;
let isMutableDesination: boolean;
let destinationType: 'context' | 'mutable' | null = null;
switch (intoKind) {
case ValueKind.Context:
case ValueKind.Mutable:
case ValueKind.MaybeFrozen: {
isMutableDesination = true;
case ValueKind.Context: {
destinationType = 'context';
break;
}
default: {
isMutableDesination = false;
case ValueKind.Mutable:
case ValueKind.MaybeFrozen: {
destinationType = 'mutable';
break;
}
}
const fromKind = state.kind(effect.from).kind;
let isMutableReferenceType: boolean;
let sourceType: 'context' | 'mutable' | 'frozen' | null = null;
switch (fromKind) {
case ValueKind.Context: {
sourceType = 'context';
break;
}
case ValueKind.Global:
case ValueKind.Primitive: {
isMutableReferenceType = false;
break;
}
case ValueKind.Frozen: {
isMutableReferenceType = false;
applyEffect(
context,
state,
{
kind: 'ImmutableCapture',
from: effect.from,
into: effect.into,
},
initialized,
effects,
);
sourceType = 'frozen';
break;
}
default: {
isMutableReferenceType = true;
sourceType = 'mutable';
break;
}
}
if (isMutableDesination && isMutableReferenceType) {

if (sourceType === 'frozen') {
applyEffect(
context,
state,
{
kind: 'ImmutableCapture',
from: effect.from,
into: effect.into,
},
initialized,
effects,
);
} else if (
(sourceType === 'mutable' && destinationType === 'mutable') ||
effect.kind === 'MaybeAlias'
) {
effects.push(effect);
} else if (
(sourceType === 'context' && destinationType != null) ||
(sourceType === 'mutable' && destinationType === 'context')
) {
applyEffect(
context,
state,
{kind: 'MaybeAlias', from: effect.from, into: effect.into},
initialized,
effects,
);
}
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -779,7 +779,13 @@ class AliasingState {
if (edge.index >= index) {
break;
}
queue.push({place: edge.node, transitive, direction: 'forwards', kind});
queue.push({
place: edge.node,
transitive,
direction: 'forwards',
// Traversing a maybeAlias edge always downgrades to conditional mutation
kind: edge.kind === 'maybeAlias' ? MutationKind.Conditional : kind,
});
}
for (const [alias, when] of node.createdFrom) {
if (when >= index) {
Expand Down Expand Up @@ -807,7 +813,12 @@ class AliasingState {
if (when >= index) {
continue;
}
queue.push({place: alias, transitive, direction: 'backwards', kind});
queue.push({
place: alias,
transitive,
direction: 'backwards',
kind,
});
}
/**
* MaybeAlias indicates potential data flow from unknown function calls,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ function emitSelectorFn(env: Environment, keys: Array<string>): Instruction {
const fn: HIRFunction = {
loc: GeneratedSource,
id: null,
nameHint: null,
fnType: 'Other',
env,
params: [obj],
Expand All @@ -275,6 +276,7 @@ function emitSelectorFn(env: Environment, keys: Array<string>): Instruction {
value: {
kind: 'FunctionExpression',
name: null,
nameHint: null,
loweredFunc: {
func: fn,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ export function outlineFunctions(
) {
const loweredFunc = value.loweredFunc.func;

const id = fn.env.generateGloballyUniqueIdentifierName(loweredFunc.id);
const id = fn.env.generateGloballyUniqueIdentifierName(
loweredFunc.id ?? loweredFunc.nameHint,
);
loweredFunc.id = id.value;

fn.env.outlineFunction(loweredFunc, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ function emitOutlinedFn(
const fn: HIRFunction = {
loc: GeneratedSource,
id: null,
nameHint: null,
fnType: 'Other',
env,
params: [propsObj],
Expand Down
Loading
Loading