Conversation
|
|
||
| /// Index of a basic block inside a [`ControlFlowGraph`]. | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] | ||
| pub struct BlockId(u32); |
There was a problem hiding this comment.
Could we get away with u16? How large does a method need to be to produce so many blocks that we exceed the 65k?
Maybe we can save a bit of memory.
| /// The unique entry block. Method execution starts here. | ||
| pub const ENTRY: Self = Self(0); | ||
| /// The unique exit block. Every terminating path eventually reaches it. | ||
| pub const EXIT: Self = Self(1); |
There was a problem hiding this comment.
Question for my understanding: if a method has multiple return statements, do their blocks all point to the same exit?
Also, this might be worth documenting.
| /// An instruction whose semantics have not been lowered yet. | ||
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| pub struct Instruction { | ||
| /// The corresponding byte range in the Ruby source. | ||
| location: Offset, | ||
| } | ||
|
|
||
| impl Instruction { | ||
| #[must_use] | ||
| pub const fn new(location: Offset) -> Self { | ||
| Self { location } | ||
| } | ||
|
|
||
| #[must_use] | ||
| pub const fn location(&self) -> &Offset { | ||
| &self.location | ||
| } | ||
| } |
There was a problem hiding this comment.
Not necessary for this PR, but I imagine instructions are going to grow a lot, so I'd move it to its own file under cfg/.
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| pub struct Instruction { | ||
| /// The corresponding byte range in the Ruby source. | ||
| location: Offset, |
There was a problem hiding this comment.
Let's call this offset. Location is a different concept so I'm worried it might cause confusion.
| /// The block that executes next. | ||
| target: BlockId, | ||
| /// The Ruby source range responsible for the transfer. | ||
| location: Offset, |
There was a problem hiding this comment.
Two things: first is the same as my previous comment, let's call this offset.
Second: are we duplicating the offsets for the same instruction? I'm trying to build my mental model of how the blocks connect exactly.
Consider this
def foo
blah = true
# The if statement here causes the branching. Is this if statement
# stored exclusively as a block terminator or is it also an instruction
# that belongs to the first block of `foo`?
if blah
else
end
end| } | ||
| } | ||
|
|
||
| /// A maximal straight-line instruction sequence with one terminator. |
There was a problem hiding this comment.
I think the maximal straight-line part may not be clear to everyone, although I do like the concise way of describing it. Can we expand a bit on it?
| } | ||
|
|
||
| struct Builder { | ||
| blocks: Vec<BasicBlock>, |
There was a problem hiding this comment.
Here we're building the blocks vector and then instantiating the Cfg with the complete list. No strong preference, but what do you think about having the builder hold a CFG instance that gets mutated instead?
Basically, something like this
self.cfg.add_block(...)| } | ||
|
|
||
| /// Appends an empty block with the next sequential ID. | ||
| fn fresh_block(&mut self) -> BlockId { |
There was a problem hiding this comment.
Nit:
| fn fresh_block(&mut self) -> BlockId { | |
| fn new_block(&mut self) -> BlockId { |
| } | ||
|
|
||
| /// Adds an instruction to the current block. | ||
| fn emit(&mut self, location: Offset) { |
There was a problem hiding this comment.
Nit:
| fn emit(&mut self, location: Offset) { | |
| fn emit_instruction(&mut self, location: Offset) { |
| }; | ||
| } | ||
| } | ||
| ControlFlowGraph::new(self.blocks) |
There was a problem hiding this comment.
If my understanding is correct, we build one CFG per file that we visit, which may contain multiple methods.
Considering we want to parallelize the analysis of the CFG, should we instead produce a new CFG for each statements node (a.k.a. method body)?
2 participants
Why
Rubydex needs a control-flow graph before it can model instruction semantics, local-variable flow, reachability, and type inference.
This PR introduces the minimum CFG topology needed to start that work. The representation follows Sorbet’s non-SSA CFG structure but does not attempt full Sorbet compatibility yet.
What changed
Added a CFG model with:
BlockIdsBasicBlockJump,Branch, andExitterminatorsAdded a Prism visitor that builds CFGs for:
ifandunlesswhileanduntilreturnThe builder preserves Ruby evaluation order and creates explicit branch, merge, loop-header, loop-exit, and unreachable blocks. These structural blocks may be empty; a future CFG simplification pass can remove unnecessary forwarding blocks.
Instructions currently contain only source locations. Instruction semantics, locals, block arguments, and type information will be introduced separately.
Testing
Builder tests render the complete CFG into a canonical textual representation. Each test verifies:
Follow-up work
This PR intentionally does not include:
break,next,redo, orretryrescue, orensurecontrol flow