[compiler] Improve name hints for outlined functions

The previous PR added name hints for anonymous functions, but didn't handle the case of outlined functions. Here we do some cleanup around function `id` and name hints:
* Make `HIRFunction.id` a ValidatedIdentifierName, which involved some cleanup of the validation helpers
* Add `HIRFunction.nameHint: string` as a place to store the generated name hints which are not valid identifiers
* Update Codegen to always use the `id` as the actual function name, and only use nameHint as part of generating the object+property wrapper for debug purposes.

This ensures we don't conflate synthesized hints with real function names. Then, we also update OutlineFunctions to use the function name _or_ the nameHint as the input to generating a unique identifier. This isn't quite as nice as the object form since we lose our formatting, but it's a simple step that gives more context to the developer than `_temp` does.

Switching to output the object+property lookup form for outlined functions is a bit more involved, let's do that in a follow-up.

Author: josephsavona

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts

@@ -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});
@@ -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',

compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts

@@ -47,6 +47,7 @@ import {
   makePropertyLiteral,
   makeType,
   promoteTemporary,
+  validateIdentifierName,
 } from './HIR';
 import HIRBuilder, {Bindings, createTemporaryPlace} from './HIRBuilder';
 import {BuiltInArrayId} from './ObjectShape';
@@ -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);
   }
@@ -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
@@ -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,

compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts

@@ -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';
@@ -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;
@@ -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>;
@@ -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'
@@ -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,
+  });
 }
 
 /**
@@ -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();
 }
 
 /**

compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts

@@ -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 +=
       '(' +
@@ -558,7 +561,7 @@ export function printInstructionValue(instrValue: ReactiveValue): string {
         instrValue.loweredFunc.func.aliasingEffects
           ?.map(printAliasingEffect)
           ?.join(', ') ?? '';
-      value = `${kind} ${name} @context[${context}] @aliasingEffects=[${aliasingEffects}]\n${fn}`;
+      value = `${kind} ${name} ${instrValue.kind === 'FunctionExpression' ? (instrValue.nameHint ?? '') : ''} @context[${context}] @aliasingEffects=[${aliasingEffects}]\n${fn}`;
       break;
     }
     case 'TaggedTemplateExpression': {

compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts

@@ -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],
@@ -275,6 +276,7 @@ function emitSelectorFn(env: Environment, keys: Array<string>): Instruction {
     value: {
       kind: 'FunctionExpression',
       name: null,
+      nameHint: null,
       loweredFunc: {
         func: fn,
       },

compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineFunctions.ts

@@ -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);

compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts

@@ -364,6 +364,7 @@ function emitOutlinedFn(
   const fn: HIRFunction = {
     loc: GeneratedSource,
     id: null,
+    nameHint: null,
     fnType: 'Other',
     env,
     params: [propsObj],

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/BuildReactiveFunction.ts

@@ -44,6 +44,7 @@ export function buildReactiveFunction(fn: HIRFunction): ReactiveFunction {
   return {
     loc: fn.loc,
     id: fn.id,
+    nameHint: fn.nameHint,
     params: fn.params,
     generator: fn.generator,
     async: fn.async,

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts

@@ -43,7 +43,6 @@ import {
   ValidIdentifierName,
   getHookKind,
   makeIdentifierName,
-  validateIdentifierName,
 } from '../HIR/HIR';
 import {printIdentifier, printInstruction, printPlace} from '../HIR/PrintHIR';
 import {eachPatternOperand} from '../HIR/visitors';
@@ -62,6 +61,7 @@ export const EARLY_RETURN_SENTINEL = 'react.early_return_sentinel';
 export type CodegenFunction = {
   type: 'CodegenFunction';
   id: t.Identifier | null;
+  nameHint: string | null;
   params: t.FunctionDeclaration['params'];
   body: t.BlockStatement;
   generator: boolean;
@@ -384,6 +384,7 @@ function codegenReactiveFunction(
     type: 'CodegenFunction',
     loc: fn.loc,
     id: fn.id !== null ? t.identifier(fn.id) : null,
+    nameHint: fn.nameHint,
     params,
     body,
     generator: fn.generator,
@@ -2328,10 +2329,6 @@ function codegenInstructionValue(
         reactiveFunction,
       ).unwrap();
 
-      const validatedName =
-        instrValue.name != null
-          ? validateIdentifierName(instrValue.name)
-          : Err(null);
       if (instrValue.type === 'ArrowFunctionExpression') {
         let body: t.BlockStatement | t.Expression = fn.body;
         if (body.body.length === 1 && loweredFunc.directives.length == 0) {
@@ -2343,9 +2340,7 @@ function codegenInstructionValue(
         value = t.arrowFunctionExpression(fn.params, body, fn.async);
       } else {
         value = t.functionExpression(
-          validatedName
-            .map<t.Identifier | null>(name => t.identifier(name))
-            .unwrapOr(null),
+          instrValue.name != null ? t.identifier(instrValue.name) : null,
           fn.params,
           fn.body,
           fn.generator,
@@ -2354,10 +2349,10 @@ function codegenInstructionValue(
       }
       if (
         cx.env.config.enableNameAnonymousFunctions &&
-        validatedName.isErr() &&
-        instrValue.name != null
+        instrValue.name == null &&
+        instrValue.nameHint != null
       ) {
-        const name = instrValue.name;
+        const name = instrValue.nameHint;
         value = t.memberExpression(
           t.objectExpression([t.objectProperty(t.stringLiteral(name), value)]),
           t.stringLiteral(name),

compiler/packages/babel-plugin-react-compiler/src/Transform/NameAnonymousFunctions.ts

@@ -26,7 +26,8 @@ export function nameAnonymousFunctions(fn: HIRFunction): void {
        * nesting depth.
        */
       const name = `${prefix}${node.generatedName}]`;
-      node.fn.name = name;
+      node.fn.nameHint = name;
+      node.fn.loweredFunc.func.nameHint = name;
     }
     /**
      * Whether or not we generated a name for the function at this node,