Skip to content

Commit

Permalink
Update on "[compiler] Rewrite useContext callee"
Browse files Browse the repository at this point in the history
If a value is specified for the LowerContextAccess environment config,
we rewrite the callee from 'useContext' to the specificed value.

This will allow us run an experiment internally.

[ghstack-poisoned]
  • Loading branch information
gsathya committed Aug 7, 2024
2 parents a039384 + a3e8af7 commit 5112793
Show file tree
Hide file tree
Showing 48 changed files with 1,521 additions and 829 deletions.
52 changes: 52 additions & 0 deletions .github/workflows/compiler_prereleases.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
name: (Compiler) Publish Prereleases

on:
workflow_call:
inputs:
commit_sha:
required: true
default: ''
type: string
release_channel:
required: true
type: string
dist_tag:
required: true
type: string
secrets:
NPM_TOKEN:
required: true

env:
TZ: /usr/share/zoneinfo/America/Los_Angeles
# https://github.com/actions/cache/blob/main/tips-and-workarounds.md#cache-segment-restore-timeout
SEGMENT_DOWNLOAD_TIMEOUT_MINS: 1
GH_TOKEN: ${{ github.token }}
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}

defaults:
run:
working-directory: compiler

jobs:
publish_prerelease:
name: Publish prelease (${{ inputs.release_channel }}) ${{ inputs.commit_sha }} @${{ inputs.dist_tag }}
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
node-version-file: '.nvmrc'
cache: yarn
cache-dependency-path: compiler/yarn.lock
- name: Restore cached node_modules
uses: actions/cache@v4
id: node_modules
with:
path: "**/node_modules"
key: ${{ runner.arch }}-${{ runner.os }}-modules-${{ hashFiles('compiler/**/yarn.lock') }}
- run: yarn install --frozen-lockfile
- name: Publish packages to npm
run: |
cp ./scripts/release/ci-npmrc ~/.npmrc
scripts/release/publish.js --frfr --ci --tags ${{ inputs.dist_tag }}
21 changes: 21 additions & 0 deletions .github/workflows/compiler_prereleases_manual.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
name: (Compiler) Publish Prereleases Manual

on:
workflow_dispatch:
inputs:
prerelease_commit_sha:
required: false

env:
TZ: /usr/share/zoneinfo/America/Los_Angeles

jobs:
publish_prerelease_experimental:
name: Publish to Experimental channel
uses: facebook/react/.github/workflows/compiler_prereleases.yml@main
with:
commit_sha: ${{ inputs.prerelease_commit_sha || github.sha }}
release_channel: experimental
dist_tag: experimental
secrets:
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
20 changes: 20 additions & 0 deletions .github/workflows/compiler_prereleases_nightly.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
name: (Compiler) Publish Prereleases Nightly

on:
schedule:
# At 10 minutes past 16:00 on Mon, Tue, Wed, Thu, and Fri
- cron: 10 16 * * 1,2,3,4,5

env:
TZ: /usr/share/zoneinfo/America/Los_Angeles

jobs:
publish_prerelease_experimental:
name: Publish to Experimental channel
uses: facebook/react/.github/workflows/compiler_prereleases.yml@main
with:
commit_sha: ${{ github.sha }}
release_channel: experimental
dist_tag: experimental
secrets:
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
6 changes: 3 additions & 3 deletions .github/workflows/shared_lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ jobs:
uses: actions/cache@v4
with:
path: "**/node_modules"
key: ${{ runner.arch }}-${{ runner.os }}-modules-${{ hashFiles('yarn.lock') }}
key: ${{ runner.arch }}-${{ runner.os }}-modules-${{ hashFiles('**/yarn.lock') }}
- run: yarn install --frozen-lockfile
- run: node ./scripts/tasks/eslint

Expand All @@ -61,7 +61,7 @@ jobs:
uses: actions/cache@v4
with:
path: "**/node_modules"
key: ${{ runner.arch }}-${{ runner.os }}-modules-${{ hashFiles('yarn.lock') }}
key: ${{ runner.arch }}-${{ runner.os }}-modules-${{ hashFiles('**/yarn.lock') }}
- run: yarn install --frozen-lockfile
- run: ./scripts/ci/check_license.sh

Expand All @@ -79,6 +79,6 @@ jobs:
uses: actions/cache@v4
with:
path: "**/node_modules"
key: ${{ runner.arch }}-${{ runner.os }}-modules-${{ hashFiles('yarn.lock') }}
key: ${{ runner.arch }}-${{ runner.os }}-modules-${{ hashFiles('**/yarn.lock') }}
- run: yarn install --frozen-lockfile
- run: ./scripts/ci/test_print_warnings.sh
1 change: 1 addition & 0 deletions compiler/crates/react_hir/src/merge_consecutive_blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ impl MergedBlocks {

#[derive(Debug, Error)]
#[error("Expected predecessor {predecessor} to exist")]
#[allow(dead_code)]
pub struct ExpectedPredecessorToExist {
predecessor: BlockId,
}
Expand Down
1 change: 1 addition & 0 deletions compiler/crates/react_semantic_analysis/src/scope_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ impl<'m> std::fmt::Debug for ScopeView<'m> {
}

#[derive(Clone, Copy)]
#[allow(dead_code)]
pub struct LabelView<'m> {
#[allow(dead_code)]
pub(crate) manager: &'m ScopeManager,
Expand Down
6 changes: 4 additions & 2 deletions compiler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@
"snap": "yarn workspace babel-plugin-react-compiler run snap",
"snap:build": "yarn workspace snap run build",
"postinstall": "perl -p -i -e 's/react\\.element/react.transitional.element/' packages/snap/node_modules/fbt/lib/FbtReactUtil.js && perl -p -i -e 's/didWarnAboutUsingAct = false;/didWarnAboutUsingAct = true;/' packages/babel-plugin-react-compiler/node_modules/react-dom/cjs/react-dom-test-utils.development.js",
"npm:publish": "node scripts/release/publish-manual"
"npm:publish": "node scripts/release/publish"
},
"dependencies": {
"fs-extra": "^4.0.2"
},
"dependencies": {},
"devDependencies": {
"@rollup/plugin-commonjs": "^25.0.7",
"@rollup/plugin-json": "^6.1.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ export const defaultOptions: PluginOptions = {
noEmit: false,
runtimeModule: null,
eslintSuppressionRules: null,
flowSuppressions: false,
flowSuppressions: true,
ignoreUseNoForget: false,
sources: filename => {
return filename.indexOf('node_modules') === -1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,11 @@ import {flattenScopesWithHooksOrUseHIR} from '../ReactiveScopes/FlattenScopesWit
import {pruneAlwaysInvalidatingScopes} from '../ReactiveScopes/PruneAlwaysInvalidatingScopes';
import pruneInitializationDependencies from '../ReactiveScopes/PruneInitializationDependencies';
import {stabilizeBlockIds} from '../ReactiveScopes/StabilizeBlockIds';
import {eliminateRedundantPhi, enterSSA, leaveSSA} from '../SSA';
import {
eliminateRedundantPhi,
enterSSA,
rewriteInstructionKindsBasedOnReassignment,
} from '../SSA';
import {inferTypes} from '../TypeInference';
import {
logCodegenFunction,
Expand All @@ -98,6 +102,7 @@ import {
} from '../Validation';
import {validateLocalsNotReassignedAfterRender} from '../Validation/ValidateLocalsNotReassignedAfterRender';
import {outlineFunctions} from '../Optimization/OutlineFunctions';
import {propagatePhiTypes} from '../TypeInference/PropagatePhiTypes';
import {lowerContextAccess} from '../Optimization/LowerContextAccess';

export type CompilerPipelineValue =
Expand Down Expand Up @@ -242,8 +247,19 @@ function* runWithEnvironment(
inferReactivePlaces(hir);
yield log({kind: 'hir', name: 'InferReactivePlaces', value: hir});

leaveSSA(hir);
yield log({kind: 'hir', name: 'LeaveSSA', value: hir});
rewriteInstructionKindsBasedOnReassignment(hir);
yield log({
kind: 'hir',
name: 'RewriteInstructionKindsBasedOnReassignment',
value: hir,
});

propagatePhiTypes(hir);
yield log({
kind: 'hir',
name: 'PropagatePhiTypes',
value: hir,
});

inferReactiveScopeVariables(hir);
yield log({kind: 'hir', name: 'InferReactiveScopeVariables', value: hir});
Expand Down
42 changes: 37 additions & 5 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1174,11 +1174,19 @@ export type NonLocalBinding =

// Represents a user-defined variable (has a name) or a temporary variable (no name).
export type Identifier = {
/*
* unique value to distinguish a variable, since name is not guaranteed to
* exist or be unique
/**
* After EnterSSA, `id` uniquely identifies an SSA instance of a variable.
* Before EnterSSA, `id` matches `declarationId`.
*/
id: IdentifierId;

/**
* Uniquely identifies a given variable in the original program. If a value is
* reassigned in the original program each reassigned value will have a distinct
* `id` (after EnterSSA), but they will still have the same `declarationId`.
*/
declarationId: DeclarationId;

// null for temporaries. name is primarily used for debugging.
name: IdentifierName | null;
// The range for which this variable is mutable
Expand Down Expand Up @@ -1212,6 +1220,7 @@ export function makeTemporaryIdentifier(
return {
id,
name: null,
declarationId: makeDeclarationId(id),
mutableRange: {start: makeInstructionId(0), end: makeInstructionId(0)},
scope: null,
type: makeType(),
Expand Down Expand Up @@ -1239,6 +1248,9 @@ export function makeIdentifierName(name: string): ValidatedIdentifier {

/**
* Given an unnamed identifier, promote it to a named identifier.
*
* Note: this uses the identifier's DeclarationId to ensure that all
* instances of the same declaration will have the same name.
*/
export function promoteTemporary(identifier: Identifier): void {
CompilerError.invariant(identifier.name === null, {
Expand All @@ -1249,7 +1261,7 @@ export function promoteTemporary(identifier: Identifier): void {
});
identifier.name = {
kind: 'promoted',
value: `#t${identifier.id}`,
value: `#t${identifier.declarationId}`,
};
}

Expand All @@ -1260,6 +1272,9 @@ export function isPromotedTemporary(name: string): boolean {
/**
* Given an unnamed identifier, promote it to a named identifier, distinguishing
* it as a value that needs to be capitalized since it appears in JSX element tag position
*
* Note: this uses the identifier's DeclarationId to ensure that all
* instances of the same declaration will have the same name.
*/
export function promoteTemporaryJsxTag(identifier: Identifier): void {
CompilerError.invariant(identifier.name === null, {
Expand All @@ -1270,7 +1285,7 @@ export function promoteTemporaryJsxTag(identifier: Identifier): void {
});
identifier.name = {
kind: 'promoted',
value: `#T${identifier.id}`,
value: `#T${identifier.declarationId}`,
};
}

Expand Down Expand Up @@ -1508,6 +1523,23 @@ export function makeIdentifierId(id: number): IdentifierId {
return id as IdentifierId;
}

/*
* Simulated opaque type for IdentifierId to prevent using normal numbers as ids
* accidentally.
*/
const opageDeclarationId = Symbol();
export type DeclarationId = number & {[opageDeclarationId]: 'DeclarationId'};

export function makeDeclarationId(id: number): DeclarationId {
CompilerError.invariant(id >= 0 && Number.isInteger(id), {
reason: 'Expected declaration id to be a non-negative integer',
description: null,
loc: null,
suggestions: null,
});
return id as DeclarationId;
}

/*
* Simulated opaque type for InstructionId to prevent using normal numbers as ids
* accidentally.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
Terminal,
VariableBinding,
makeBlockId,
makeDeclarationId,
makeIdentifierName,
makeInstructionId,
makeTemporaryIdentifier,
Expand Down Expand Up @@ -320,6 +321,7 @@ export default class HIRBuilder {
const id = this.nextIdentifierId;
const identifier: Identifier = {
id,
declarationId: makeDeclarationId(id),
name: makeIdentifierName(name),
mutableRange: {
start: makeInstructionId(0),
Expand Down Expand Up @@ -897,3 +899,16 @@ export function createTemporaryPlace(
loc: GeneratedSource,
};
}

/**
* Clones an existing Place, returning a new temporary Place that shares the
* same metadata properties as the original place (effect, reactive flag, type)
* but has a new, temporary Identifier.
*/
export function clonePlaceToTemporary(env: Environment, place: Place): Place {
const temp = createTemporaryPlace(env, place.loc);
temp.effect = place.effect;
temp.identifier.type = place.identifier.type;
temp.reactive = place.reactive;
return temp;
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
} from '../HIR';
import {deadCodeElimination} from '../Optimization';
import {inferReactiveScopeVariables} from '../ReactiveScopes';
import {leaveSSA} from '../SSA';
import {rewriteInstructionKindsBasedOnReassignment} from '../SSA';
import {logHIRFunction} from '../Utils/logger';
import {inferMutableContextVariables} from './InferMutableContextVariables';
import {inferMutableRanges} from './InferMutableRanges';
Expand Down Expand Up @@ -108,7 +108,7 @@ function lower(func: HIRFunction): void {
inferReferenceEffects(func, {isFunctionExpression: true});
deadCodeElimination(func);
inferMutableRanges(func);
leaveSSA(func);
rewriteInstructionKindsBasedOnReassignment(func);
inferReactiveScopeVariables(func);
inferMutableContextVariables(func);
logHIRFunction('AnalyseFunction (inner)', func);
Expand Down
Loading

0 comments on commit 5112793

Please sign in to comment.