refactor(cfg): use IndexVec for storing basic blocks #6323
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Your org has enabled the Graphite merge queue for merging into mainAdd the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
github-actions
bot
added
A-linter
This was referenced Oct 6, 2024
Contributor
Author
CodSpeed Performance ReportMerging #6323 will not alter performanceComparing Summary
|
Member
|
3 things: Why is If you are making that change, it'd be much easier to review a PR like this if the unimportant change - renaming the type - was a separate PR, so that the substantive change stands out. It's quite hard to wade through the diff in a PR like this, and figure out what's going on. If I can ask a favour, it's really helpful if you can put some description on PRs. It's doesn't need to be an essay, but just something short indicating the purpose of/motivation for the change, and any salient points about the implementation. The code usually is self-explanatory concerning the "what", but it often doesn't give you the "why". |
Contributor
Author
|
@overlookmotel thanks for the feedback. I've added a description, and it should answer your question. |
Contributor
Merge activity
|
Boshen
force-pushed
the
don/10-06-refactor_cfg_add_type_alias_for_graph
branch
from
14418b6 to
e016543
Compare
Boshen
pushed a commit
that referenced
this pull request
Oct 7, 2024
Use an `IndexVec` when storing basic blocks. This makes the link between nodes in `.graph` and elements of `.basic_blocks` more clear. I had to rename `BasicBlockId` to `BlockNodeId` to avoid a name collision. I wasn't sure what else to name the `Idx` type for the basic blocks vec.
Boshen
force-pushed
the
don/10-06-refactor_cfg_use_indexvec_for_storing_basic_blocks
branch
from
b9914d3 to
ddc0a3d
Compare
DonIsaac
force-pushed
the
don/10-06-refactor_cfg_add_type_alias_for_graph
branch
from
e016543 to
7f4eea4
Compare
DonIsaac
added a commit
that referenced
this pull request
Oct 7, 2024
Use an `IndexVec` when storing basic blocks. This makes the link between nodes in `.graph` and elements of `.basic_blocks` more clear. I had to rename `BasicBlockId` to `BlockNodeId` to avoid a name collision. I wasn't sure what else to name the `Idx` type for the basic blocks vec.
DonIsaac
force-pushed
the
don/10-06-refactor_cfg_use_indexvec_for_storing_basic_blocks
branch
from
ddc0a3d to
148d9a9
Compare
DonIsaac
force-pushed
the
don/10-06-refactor_cfg_add_type_alias_for_graph
branch
from
7f4eea4 to
67104b6
Compare
DonIsaac
added a commit
that referenced
this pull request
Oct 7, 2024
Use an `IndexVec` when storing basic blocks. This makes the link between nodes in `.graph` and elements of `.basic_blocks` more clear. I had to rename `BasicBlockId` to `BlockNodeId` to avoid a name collision. I wasn't sure what else to name the `Idx` type for the basic blocks vec.
DonIsaac
force-pushed
the
don/10-06-refactor_cfg_use_indexvec_for_storing_basic_blocks
branch
from
148d9a9 to
3fb9321
Compare
DonIsaac
changed the base branch from
don/10-06-refactor_cfg_add_type_alias_for_graph
to
graphite-base/6323
DonIsaac
added a commit
that referenced
this pull request
Oct 7, 2024
Use an `IndexVec` when storing basic blocks. This makes the link between nodes in `.graph` and elements of `.basic_blocks` more clear. I had to rename `BasicBlockId` to `BlockNodeId` to avoid a name collision. I wasn't sure what else to name the `Idx` type for the basic blocks vec.
DonIsaac
force-pushed
the
graphite-base/6323
branch
from
67104b6 to
a1e0d30
Compare
DonIsaac
force-pushed
the
don/10-06-refactor_cfg_use_indexvec_for_storing_basic_blocks
branch
from
3fb9321 to
3ff332b
Compare
DonIsaac
added a commit
that referenced
this pull request
Oct 7, 2024
Use an `IndexVec` when storing basic blocks. This makes the link between nodes in `.graph` and elements of `.basic_blocks` more clear. I had to rename `BasicBlockId` to `BlockNodeId` to avoid a name collision. I wasn't sure what else to name the `Idx` type for the basic blocks vec.
DonIsaac
force-pushed
the
don/10-06-refactor_cfg_use_indexvec_for_storing_basic_blocks
branch
from
3ff332b to
a7f92bb
Compare
DonIsaac
added a commit
that referenced
this pull request
Oct 7, 2024
Use an `IndexVec` when storing basic blocks. This makes the link between nodes in `.graph` and elements of `.basic_blocks` more clear. I had to rename `BasicBlockId` to `BlockNodeId` to avoid a name collision. I wasn't sure what else to name the `Idx` type for the basic blocks vec.
DonIsaac
force-pushed
the
don/10-06-refactor_cfg_use_indexvec_for_storing_basic_blocks
branch
from
a7f92bb to
b848846
Compare
Use an `IndexVec` when storing basic blocks. This makes the link between nodes in `.graph` and elements of `.basic_blocks` more clear. I had to rename `BasicBlockId` to `BlockNodeId` to avoid a name collision. I wasn't sure what else to name the `Idx` type for the basic blocks vec.
DonIsaac
force-pushed
the
don/10-06-refactor_cfg_use_indexvec_for_storing_basic_blocks
branch
from
b848846 to
40932f7
Compare
graphite-app
bot
deleted the
don/10-06-refactor_cfg_use_indexvec_for_storing_basic_blocks
branch
Boshen
added a commit
that referenced
this pull request
Oct 8, 2024
## [0.31.0] - 2024-10-08 - 01b878e parser: [**BREAKING**] Use `BindingIdentifier` for `namespace` declaration names (#6003) (DonIsaac) - 95ca01c cfg: [**BREAKING**] Make BasicBlock::unreachable private (#6321) (DonIsaac) - 020bb80 codegen: [**BREAKING**] Change to `CodegenReturn::code` and `CodegenReturn::map` (#6310) (Boshen) - 409dffc traverse: [**BREAKING**] `generate_uid` return a `BoundIdentifier` (#6294) (overlookmotel) - 5a73a66 regular_expression: [**BREAKING**] Simplify public APIs (#6262) (leaysgur) - 32d972e parser: [**BREAKING**] Treat unambiguous files containing TS export assignments as modules (#6253) (overlookmotel) - 4f6bc79 transformer: [**BREAKING**] Remove `source_type` param from `Transformer::new` (#6251) (overlookmotel) - afc3ccb napi/transform: [**BREAKING**] Rename `TransformOptions::react` to `jsx`. (#6211) (Boshen) - 82ab689 transformer,minifier: [**BREAKING**] Move define and inject plugin from minifier to transformer (#6199) (Boshen) ### Features - fa4d505 cfg: Derive more base traits for CFG blocks (#6320) (DonIsaac) - 14275b1 cfg: Color-code edges in CFG dot diagrams (#6314) (DonIsaac) - 7566c2d data_structures: Add `as_slice` + `as_mut_slice` methods to stacks (#6216) (overlookmotel) - c3c3447 data_structures: Add `oxc_data_structures` crate; add stack (#6206) (Boshen) - e304e8c minifier: Minify exponential arithmetic operation. (#6281) (7086cmd) - f9ae70c minifier: Minify basic arithmetic calculations. (#6280) (7086cmd) - 4008afe minifier: Fold array and object constructors (#6257) (camchenry) - 115ccc9 minifier: Bitwise not in exceeded value. (#6235) (7086cmd) - ee6c850 minifier: Scaffold peephole replace known methods. (#6245) (7086cmd) - c32af57 minifier: Fold demical bitwise not for bigint. (#6233) (7086cmd) - 23b6464 minifier: Fold true / false comparison. (#6225) (7086cmd) - 585ccda minifier: Support subtraction assignment. (#6214) (7086cmd) - cca0034 minifier: Handle positive `NaN` and `Infinity`. (#6207) (7086cmd) - dac8f09 minifier: Minify unary plus negation. (#6203) (7086cmd) - 3b79e1b minifier: Evaluate bigint in fold constant (#6178) (Boshen) - abd3a9f napi/transform: Perform dce after define plugin (#6312) (Boshen) - a0ccc26 napi/transform: Add `lang` option to change source type (#6309) (Boshen) - f98e12c napi/transform: Add inject plugin (#6250) (Boshen) - 291891e napi/transform: Add `define` option (#6212) (Boshen) - 51a78d5 napi/transform: Rename all mention of React to Jsx; remove mention of `Binding` (#6198) (Boshen) - 2f888ed oxc: Add napi transform options (#6268) (Boshen) - 8729755 oxc,napi/transform: Napi/transform use oxc compiler pipeline (#6298) (Boshen) - f6e42b6 sourcemap: Add support for sourcemap debug IDs (#6221) (Tim Fish) - 9e62396 syntax_operations: Add crate `oxc_syntax_operations` (#6202) (Boshen) - cf20f3a transformer: Exponentiation transform: support private fields (#6345) (overlookmotel) ### Bug Fixes - 84b2d07 codegen: Converts line comment to block comment if it is a `PURE` comment (#6356) (Dunqing) - e9eeae0 isolated-declarations: False positive for function with a type asserted parameters (#6181) (Dunqing) - d953a6b minifier: Correct the reference link (#6283) (dalaoshu) - 37cbabb minifier: Should not handle the strict operation for bool comparison. (#6261) (7086cmd) - e29c067 minifier: Handle exceeded shifts. (#6237) (7086cmd) - 294da86 napi/transform: Fix index.d.ts (Boshen) - 9736aa0 oxc_transformer: Define `import.meta` and `import.meta.*` (#6277) (IWANABETHATGUY) - 6159560 parser: String `ImportSpecifier`s for type imports (#6352) (DonIsaac) - 1380d8b parser: Should regard comments where after `=` as leading comments of next token (#6355) (Dunqing) - 2bcd12a transformer: Exponentiation transform: fix reference flags (#6330) (overlookmotel) - 28cbfa7 transformer: Exponentiation transform: fix temp var names (#6329) (overlookmotel) - 3a4bcc7 transformer: Exponentiation transform: fix temp var names (#6318) (overlookmotel) - ccb7bdc transformer: Exponentiation transform: do not replace object when private property (#6313) (overlookmotel) - 56d50cf transformer: Exponentiation transform: do not assume `Math` is not a local var (#6302) (overlookmotel) - bd81c51 transformer: Exponentiation transform: fix duplicate symbols (#6300) (overlookmotel) - 06797b6 transformer: Logical assignment operator transform: fix reference IDs (#6289) (overlookmotel) - 4b42047 transformer: Fix memory leak in `ReplaceGlobalDefines` (#6224) (overlookmotel) - a28926f transformer: Fix inserting `require` with `front` option (#6188) (overlookmotel) - b92fe84 transformer: `NonEmptyStack::push` write value before updating cursor (#6169) (overlookmotel) ### Performance - 5db9b30 allocator: Use lower bound of size hint when creating Vecs from an iterator (#6194) (DonIsaac) - 788e444 transformer: Parse options from comments only once (#6152) (overlookmotel) - da2b2a4 transformer: Look up `SymbolId` for `require` only once (#6192) (overlookmotel) - 40bd919 transformer: Faster parsing JSX pragmas from comments (#6151) (overlookmotel) ### Documentation - eb1d0b8 transformer: Exponentiation transform: update doc comments (#6315) (overlookmotel) - c7636d7 traverse: Remove erroneous doc comment (#6328) (overlookmotel) ### Refactor - f7d1136 allocator: Remove unnecessary `Vec` impl (#6213) (overlookmotel) - 40932f7 cfg: Use IndexVec for storing basic blocks (#6323) (DonIsaac) - a1e0d30 cfg: Add type alias for Graph (#6322) (DonIsaac) - 7672793 cfg: Move block data types to separate file (#6319) (DonIsaac) - cc57541 data_structures: `NonEmptyStack::len` hint that `len` is never 0 (#6220) (overlookmotel) - 147a5d5 data_structures: Remove `is_empty` methods for non-empty stacks (#6219) (overlookmotel) - 61805fd data_structures: Add debug assertion to `SparseStack` (#6218) (overlookmotel) - dbfa0bc data_structures: Add `len` method to `StackCommon` trait (#6215) (overlookmotel) - ac5a23f minifier: Use ctx.ast.vec instead of Vec::new. (#6331) (7086cmd) - 1cee207 minifier: Some boilerplate work for PeepholeFoldConstants (#6054) (Boshen) - 5b5daec napi: Use vitest (#6307) (Boshen) - 58a8615 napi/transform: Remove context (#6306) (Boshen) - 099ff3a napi/transform: Remove "Binding" from types; fix type error (#6260) (Boshen) - 54c1c53 napi/transform: Remove a call on `TransformOptions::clone` (#6210) (Boshen) - aa0dbb6 oxc: Add `napi` feature, change napi parser to use `oxc` crate (#6265) (Boshen) - 3b53dd4 parser: Provide better error messages for `const` modifiers on class elements (#6353) (DonIsaac) - acab777 regular_expression: Misc fixes (#6234) (leaysgur) - bdd9e92 semantic: Rename vars from `ast_node_id` to `node_id` (#6304) (overlookmotel) - d110700 semantic: Dereference IDs as quickly as possible (#6303) (overlookmotel) - 03bc041 syntax: Remove some unsafe code creating IDs (#6324) (overlookmotel) - bd5fb5a transformer: Exponentiation transform: rename methods (#6344) (overlookmotel) - 4aa4e6b transformer: Exponentiation transform: do not wrap in `SequenceExpression` if not needed (#6343) (overlookmotel) - a15235a transformer: Exponentiation transform: no cloning (#6338) (overlookmotel) - 7d93b25 transformer: Exponentiation transform: split into 2 paths (#6316) (overlookmotel) - 15cc8af transformer: Exponentiation transform: break up into functions (#6301) (overlookmotel) - 7f5a94b transformer: Use `Option::get_or_insert_with` (#6299) (overlookmotel) - 0cea6e9 transformer: Exponentiation transform: reduce identifier cloning (#6297) (overlookmotel) - ac7a3ed transformer: Logical assignment transform: reduce identifier cloning (#6296) (overlookmotel) - 527f7c8 transformer: Nullish coalescing transform: no cloning identifier references (#6295) (overlookmotel) - 7b62966 transformer: Move `BoundIdentifier` into `oxc_traverse` crate (#6293) (overlookmotel) - c7fbf68 transformer: Logical assignment operator transform: no cloning identifier references (#6290) (overlookmotel) - f0a74ca transformer: Prefer `create_bound_reference_id` to `create_reference_id` (#6282) (overlookmotel) - ba3e85b transformer: Fix spelling (#6279) (overlookmotel) - bc757c8 transformer: Move functionality of common transforms into stores (#6243) (overlookmotel) - 1c31932 transformer: Rename var in `VarDeclarations` common transform (#6242) (overlookmotel) - 0400ff9 transformer: `VarDeclarations` common transform: check if at top level with `ctx.parent()` (#6231) (overlookmotel) - 235cdba transformer: Use AstBuilder instance from TraverseCtx (#6209) (overlookmotel) - a7ed29e transformer: Insert `import` statement or `require` depending on source type (#6191) (overlookmotel) - 4c63f0e transformer: Rename methods (#6190) (overlookmotel) - 900cb46 transformer: Convert `ModuleImports` into common transform (#6186) (overlookmotel) - 00e2802 transformer: Introduce `TopLevelStatements` common transform (#6185) (overlookmotel) - 70d4c56 transformer: Rename `VarDeclarationsStore` methods (#6184) (overlookmotel) - 81be545 transformer: Export `var_declarations` module from `common` module (#6183) (overlookmotel) - 02fedf5 transformer: Shorten import (#6180) (overlookmotel) - f2ac584 transformer: Use TraverseCtx's ast in ModuleImports (#6175) (Dunqing) - 21b08ba transformer: Shared `VarDeclarations` (#6170) (overlookmotel) - 0dd9a2e traverse: Add helper methods to `BoundIdentifier` (#6341) (overlookmotel) - c0e2fef traverse: Function to get var name from node (#6317) (overlookmotel) - adc5381 traverse: `TraverseAncestry` use `NonEmptyStack` (#6217) (overlookmotel) ### Testing - 964d71e minifier: Add arithmetic tests for fold constants. (#6269) (7086cmd) - fcb4651 minifier: Enable null comparison with bigint. (#6252) (7086cmd) - d4f2ee9 transformer: Tidy up transform checker (#6287) (overlookmotel) - 0f5afd7 transformer: Transform checker output symbol name for mismatches (#6286) (overlookmotel) --------- Co-authored-by: Boshen <[email protected]> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use an
IndexVecwhen storing basic blocks. This makes the link between nodes in.graphand elements of.basic_blocksmore clear. I had to renameBasicBlockIdtoBlockNodeIdto avoid a name collision. I wasn't sure what else to name theIdxtype for the basic blocks vec.