Skip to content

Commit

Permalink
Update (base update)
Browse files Browse the repository at this point in the history 
[ghstack-poisoned]
  • Loading branch information
@mofeiZ
mofeiZ committed Sep 26, 2024
1 parent b27edfb commit 2e6b5cd
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 53 deletions.
88 changes: 50 additions & 38 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,10 @@ export function collectHoistablePropertyLoads(
temporaries: ReadonlyMap<IdentifierId, ReactiveScopeDependency>,
optionals: ReadonlyMap<BlockId, ReactiveScopeDependency>,
): ReadonlyMap<ScopeId, BlockInfo> {
const tree = new Tree();
const registry = new PropertyPathRegistry();

const nodes = collectNonNullsInBlocks(fn, temporaries, optionals, tree);
propagateNonNull(fn, nodes, tree);
const nodes = collectNonNullsInBlocks(fn, temporaries, optionals, registry);
propagateNonNull(fn, nodes, registry);

const nodesKeyedByScopeId = new Map<ScopeId, BlockInfo>();
for (const [_, block] of fn.body.blocks) {
Expand All @@ -94,37 +94,37 @@ export function collectHoistablePropertyLoads(

export type BlockInfo = {
block: BasicBlock;
assumedNonNullObjects: ReadonlySet<PropertyLoadNode>;
assumedNonNullObjects: ReadonlySet<PropertyPathNode>;
};

/**
* Tree data structure to dedupe property loads (e.g. a.b.c)
* PropertyLoadRegistry data structure to dedupe property loads (e.g. a.b.c)
* and make computing sets intersections simpler.
*/
type RootNode = {
properties: Map<string, PropertyLoadNode>;
optionalProperties: Map<string, PropertyLoadNode>;
properties: Map<string, PropertyPathNode>;
optionalProperties: Map<string, PropertyPathNode>;
parent: null;
// Recorded to make later computations simpler
fullPath: ReactiveScopeDependency;
hasOptional: boolean;
root: IdentifierId;
};

type PropertyLoadNode =
type PropertyPathNode =
| {
properties: Map<string, PropertyLoadNode>;
optionalProperties: Map<string, PropertyLoadNode>;
parent: PropertyLoadNode;
properties: Map<string, PropertyPathNode>;
optionalProperties: Map<string, PropertyPathNode>;
parent: PropertyPathNode;
fullPath: ReactiveScopeDependency;
hasOptional: boolean;
}
| RootNode;

class Tree {
class PropertyPathRegistry {
roots: Map<IdentifierId, RootNode> = new Map();

getOrCreateIdentifier(identifier: Identifier): PropertyLoadNode {
getOrCreateIdentifier(identifier: Identifier): PropertyPathNode {
/**
* Reads from a statically scoped variable are always safe in JS,
* with the exception of TDZ (not addressed by this pass).
Expand All @@ -149,9 +149,9 @@ class Tree {
}

static getOrCreatePropertyEntry(
parent: PropertyLoadNode,
parent: PropertyPathNode,
entry: DependencyPathEntry,
): PropertyLoadNode {
): PropertyPathNode {
const map = entry.optional ? parent.optionalProperties : parent.properties;
let child = map.get(entry.property);
if (child == null) {
Expand All @@ -170,7 +170,7 @@ class Tree {
return child;
}

getOrCreateProperty(n: ReactiveScopeDependency): PropertyLoadNode {
getOrCreateProperty(n: ReactiveScopeDependency): PropertyPathNode {
/**
* We add ReactiveScopeDependencies according to instruction ordering,
* so all subpaths of a PropertyLoad should already exist
Expand All @@ -181,18 +181,24 @@ class Tree {
return currNode;
}
for (let i = 0; i < n.path.length - 1; i++) {
currNode = Tree.getOrCreatePropertyEntry(currNode, n.path[i]);
currNode = PropertyPathRegistry.getOrCreatePropertyEntry(
currNode,
n.path[i],
);
}

return Tree.getOrCreatePropertyEntry(currNode, n.path.at(-1)!);
return PropertyPathRegistry.getOrCreatePropertyEntry(
currNode,
n.path.at(-1)!,
);
}
}

function pushPropertyLoadNode(
node: PropertyLoadNode,
function addNonNullPropertyPath(
node: PropertyPathNode,
instrId: InstructionId,
knownImmutableIdentifiers: Set<IdentifierId>,
result: Set<PropertyLoadNode>,
result: Set<PropertyPathNode>,
): void {
const object = node.fullPath.identifier;
/**
Expand Down Expand Up @@ -221,7 +227,7 @@ function collectNonNullsInBlocks(
fn: HIRFunction,
temporaries: ReadonlyMap<IdentifierId, ReactiveScopeDependency>,
optionals: ReadonlyMap<BlockId, ReactiveScopeDependency>,
tree: Tree,
registry: PropertyPathRegistry,
): ReadonlyMap<BlockId, BlockInfo> {
/**
* Due to current limitations of mutable range inference, there are edge cases in
Expand All @@ -243,19 +249,19 @@ function collectNonNullsInBlocks(
* Known non-null objects such as functional component props can be safely
* read from any block.
*/
const knownNonNullIdentifiers = new Set<PropertyLoadNode>();
const knownNonNullIdentifiers = new Set<PropertyPathNode>();
if (
fn.env.config.enablePropagateDepsInHIR === 'enabled_with_optimizations' &&
fn.fnType === 'Component' &&
fn.params.length > 0 &&
fn.params[0].kind === 'Identifier'
) {
const identifier = fn.params[0].identifier;
knownNonNullIdentifiers.add(tree.getOrCreateIdentifier(identifier));
knownNonNullIdentifiers.add(registry.getOrCreateIdentifier(identifier));
}
const nodes = new Map<BlockId, BlockInfo>();
for (const [_, block] of fn.body.blocks) {
const assumedNonNullObjects = new Set<PropertyLoadNode>(
const assumedNonNullObjects = new Set<PropertyPathNode>(
knownNonNullIdentifiers,
);

Expand All @@ -265,7 +271,9 @@ function collectNonNullsInBlocks(
});
const maybeOptionalChain = optionals.get(block.id);
if (maybeOptionalChain != null) {
assumedNonNullObjects.add(tree.getOrCreateProperty(maybeOptionalChain));
assumedNonNullObjects.add(
registry.getOrCreateProperty(maybeOptionalChain),
);
continue;
}
for (const instr of block.instructions) {
Expand All @@ -274,8 +282,8 @@ function collectNonNullsInBlocks(
identifier: instr.value.object.identifier,
path: [],
};
const propertyNode = tree.getOrCreateProperty(source);
pushPropertyLoadNode(
const propertyNode = registry.getOrCreateProperty(source);
addNonNullPropertyPath(
propertyNode,
instr.id,
knownImmutableIdentifiers,
Expand All @@ -288,8 +296,8 @@ function collectNonNullsInBlocks(
const source = instr.value.value.identifier.id;
const sourceNode = temporaries.get(source);
if (sourceNode != null) {
pushPropertyLoadNode(
tree.getOrCreateProperty(sourceNode),
addNonNullPropertyPath(
registry.getOrCreateProperty(sourceNode),
instr.id,
knownImmutableIdentifiers,
assumedNonNullObjects,
Expand All @@ -302,8 +310,8 @@ function collectNonNullsInBlocks(
const source = instr.value.object.identifier.id;
const sourceNode = temporaries.get(source);
if (sourceNode != null) {
pushPropertyLoadNode(
tree.getOrCreateProperty(sourceNode),
addNonNullPropertyPath(
registry.getOrCreateProperty(sourceNode),
instr.id,
knownImmutableIdentifiers,
assumedNonNullObjects,
Expand All @@ -318,7 +326,7 @@ function collectNonNullsInBlocks(
function propagateNonNull(
fn: HIRFunction,
nodes: ReadonlyMap<BlockId, BlockInfo>,
tree: Tree,
registry: PropertyPathRegistry,
): void {
const blockSuccessors = new Map<BlockId, Set<BlockId>>();
const terminalPreds = new Set<BlockId>();
Expand Down Expand Up @@ -404,7 +412,7 @@ function propagateNonNull(

const prevObjects = assertNonNull(nodes.get(nodeId)).assumedNonNullObjects;
const mergedObjects = Set_union(prevObjects, neighborAccesses);
reduceMaybeOptionalChains(mergedObjects, tree);
reduceMaybeOptionalChains(mergedObjects, registry);

assertNonNull(nodes.get(nodeId)).assumedNonNullObjects = mergedObjects;
traversalState.set(nodeId, 'done');
Expand Down Expand Up @@ -471,8 +479,8 @@ export function assertNonNull<T extends NonNullable<U>, U>(
* with <base>.PROPERTY_STRING
*/
function reduceMaybeOptionalChains(
nodes: Set<PropertyLoadNode>,
tree: Tree,
nodes: Set<PropertyPathNode>,
registry: PropertyPathRegistry,
): void {
let optionalChainNodes = Set_filter(nodes, n => n.hasOptional);
if (optionalChainNodes.size === 0) {
Expand All @@ -485,15 +493,19 @@ function reduceMaybeOptionalChains(

for (const original of optionalChainNodes) {
let {identifier, path: origPath} = original.fullPath;
let currNode: PropertyLoadNode = tree.getOrCreateIdentifier(identifier);
let currNode: PropertyPathNode =
registry.getOrCreateIdentifier(identifier);
for (let i = 0; i < origPath.length; i++) {
const entry = origPath[i];
// If the base is known to be non-null, replace with a non-optional load
const nextEntry: DependencyPathEntry =
entry.optional && knownNonNulls.has(currNode)
? {property: entry.property, optional: false}
: entry;
currNode = Tree.getOrCreatePropertyEntry(currNode, nextEntry);
currNode = PropertyPathRegistry.getOrCreatePropertyEntry(
currNode,
nextEntry,
);
}
if (currNode !== original) {
changed = true;
Expand Down
2 changes: 1 addition & 1 deletion ...mpiler/reduce-reactive-deps/jump-unpoisoned/throw-before-scope-starts.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export const FIXTURE_ENTRYPOINT = {
### Eval output
(kind: ok) [[ (exception in render) Error: throw with error! ]]
[[ (exception in render) Error: throw with error! ]]
[2]
[[ (exception in render) Error: throw with error! ]]
[[ (exception in render) TypeError: Cannot read properties of undefined (reading 'b') ]]
[null]
Expand Down
45 changes: 31 additions & 14 deletions compiler/packages/snap/src/sprout/evaluator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,38 +60,55 @@ const ExportSchema = z.object({
FIXTURE_ENTRYPOINT: EntrypointSchema,
});

const NO_ERROR_SENTINEL = Symbol();
/**
* Wraps WrapperTestComponent in an error boundary to simplify re-rendering
* when an exception is thrown.
* A simpler alternative may be to re-mount test components manually.
*/
class WrapperTestComponentWithErrorBoundary extends React.Component<
{fn: any; params: Array<any>},
{hasError: boolean; error: any}
{errorFromLastRender: any}
> {
propsErrorMap: MutableRefObject<Map<any, any>>;
/**
* Limit retries of the child component by caching seen errors.
*/
propsErrorMap: Map<any, any>;
lastProps: any | null;
// lastProps: object | null;
constructor(props: any) {
super(props);
this.state = {hasError: false, error: null};
this.propsErrorMap = React.createRef() as MutableRefObject<Map<any, any>>;
this.propsErrorMap.current = new Map();
this.lastProps = null;
this.propsErrorMap = new Map<any, any>();
this.state = {
errorFromLastRender: NO_ERROR_SENTINEL,
};
}
static getDerivedStateFromError(error: any) {
return {hasError: true, error: error};
// Reschedule a second render that immediately returns the cached error
return {errorFromLastRender: error};
}
override componentDidUpdate() {
if (this.state.hasError) {
this.setState({hasError: false, error: null});
if (this.state.errorFromLastRender !== NO_ERROR_SENTINEL) {
// Reschedule a third render that immediately returns the cached error
this.setState({errorFromLastRender: NO_ERROR_SENTINEL});
}
}
override render() {
if (this.state.hasError) {
this.propsErrorMap.current!.set(
this.props,
`[[ (exception in render) ${this.state.error?.toString()} ]]`,
);
if (
this.state.errorFromLastRender !== NO_ERROR_SENTINEL &&
this.props === this.lastProps
) {
/**
* The last render errored, cache the error message to avoid running the
* test fixture more than once
*/
const errorMsg = `[[ (exception in render) ${this.state.errorFromLastRender?.toString()} ]]`;
this.propsErrorMap.set(this.lastProps, errorMsg);
return errorMsg;
}
const cachedError = this.propsErrorMap.current!.get(this.props);
this.lastProps = this.props;
const cachedError = this.propsErrorMap.get(this.props);
if (cachedError != null) {
return cachedError;
}
Expand Down

0 comments on commit 2e6b5cd

Please sign in to comment.