Skip to content

Conversation

mofeiZ
Copy link
Contributor

@mofeiZ mofeiZ commented Sep 23, 2024

Stack from ghstack (oldest at bottom):

Rename for clarity:

  • CollectHoistablePropertyLoads:Tree -> CollectHoistablePropertyLoads:PropertyPathRegistry
    • getPropertyLoadNode -> getOrCreateProperty
    • getOrCreateRoot -> getOrCreateIdentifier
  • PropertyLoadNode -> PropertyPathNode

Refactor to CFG joining logic for CollectHoistablePropertyLoads. We now write to the same set of inferredNonNullObjects when traversing from entry and exit blocks. This is more correct, as non-nulls inferred from a forward traversal should be included when computing the backward traversal (and vice versa). This fix is needed by an edge case in #31036

Added invariant into fixed-point iteration to terminate (instead of infinite looping).

[ghstack-poisoned]
Copy link

vercel bot commented Sep 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 30, 2024 4:38pm

[ghstack-poisoned]
@mofeiZ mofeiZ changed the title [compiler][wip] Refactor HoistablePropertyLoads [compiler][wip] Renames and no-op refactor for next PR Sep 25, 2024
@mofeiZ mofeiZ changed the title [compiler][wip] Renames and no-op refactor for next PR [compiler] Renames and no-op refactor for #31037 Sep 25, 2024
@mofeiZ mofeiZ marked this pull request as ready for review September 25, 2024 23:01
[ghstack-poisoned]
[ghstack-poisoned]
result.add(curr);
curr = curr.parent;
}
result.add(node);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trust that all previous property loads were added correctly. If a property load chain ever spans multiple blocks, this is technically the correct behavior

[ghstack-poisoned]
@mofeiZ mofeiZ changed the title [compiler] Renames and no-op refactor for #31037 [compiler] Renames and no-op refactor for next PR Sep 26, 2024
[ghstack-poisoned]
@mofeiZ mofeiZ merged commit 7d77a79 into gh/mofeiZ/21/base Sep 30, 2024
19 of 20 checks passed
mofeiZ added a commit that referenced this pull request Sep 30, 2024
Rename for clarity:
- `CollectHoistablePropertyLoads:Tree` -> `CollectHoistablePropertyLoads:PropertyPathRegistry`
    - `getPropertyLoadNode` -> `getOrCreateProperty`
    - `getOrCreateRoot` -> `getOrCreateIdentifier`
- `PropertyLoadNode` -> `PropertyPathNode`

Refactor to CFG joining logic for `CollectHoistablePropertyLoads`. We now write to the same set of inferredNonNullObjects when traversing from entry and exit blocks. This is more correct, as non-nulls inferred from a forward traversal should be included when computing the backward traversal (and vice versa). This fix is needed by an edge case in #31036

Added invariant into fixed-point iteration to terminate (instead of infinite looping).

ghstack-source-id: 1e8eb2d
Pull Request resolved: #31036
@mofeiZ mofeiZ deleted the gh/mofeiZ/21/head branch September 30, 2024 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants