@samitouri / QOS-React-2 / commits / 1d2e7ee747

[rust] Use Rc<RefCell<>> for shared identifiers

Multiple Place instances can share a reference to a given Identifier in our JS implementation. For simplicity of the initial port I’m using Rc (for sharing) and RefCell (for runtime-checked mutability). This is the standard pattern for shared mutable references in Rust when you don’t need multi-threaded support. We don’t need HIR to be accessible by multiple threads so this is fine, if we do multiple threads it will be to parallelize compilation of separate functions. There are other idioms w less runtime overhead, such as Place holding an index into a separate vec of identifiers, but that would make the port much less straightforward.

Joe Savona committed Jul 6, 2023 at 09:24 UTC 1d2e7ee74706b57fb293089671153bac7e57ec93
7 files changed +59 -21
compiler/forget/crates/build-hir/src/build.rs
+3 -5
@@ -52,7 +52,7 @@ fn lower_statement<'a>(
52 env: &'a Environment<'a>,
53 builder: &mut Builder<'a>,
54 stmt: Statement,
55 - label: Option<String<'a>>,
55 + _label: Option<String<'a>>,
56 ) -> Result<(), Diagnostic> {
57 match stmt {
58 Statement::BlockStatement(stmt) => {
@@ -97,8 +97,6 @@ fn lower_statement<'a>(
97 );
98 }
99 Statement::ExpressionStatement(stmt) => {
100 - // TODO: port the logic for emitting an ExpressionStatement instr if the instr
101 - // was a logical or conditional. is that even necessary anymore?
100 lower_expression_to_temporary(env, builder, stmt.expression);
101 }
102 Statement::EmptyStatement(_) => {
@@ -175,13 +173,13 @@ fn lower_value_to_temporary<'a>(
173 return place;
174 }
175 let place = build_temporary_place(env, builder);
178 - builder.push(todo!("clone `place`"), value);
176 + builder.push(place.clone(), value);
177 return place;
178 }
179
180 /// Constructs a temporary Identifier and Place wrapper, which can be used as an Instruction lvalue
181 /// or other places where a temporary target is required
184 -fn build_temporary_place<'a>(env: &'a Environment<'a>, builder: &mut Builder<'a>) -> Place<'a> {
182 +fn build_temporary_place<'a>(_env: &'a Environment<'a>, builder: &mut Builder<'a>) -> Place<'a> {
183 Place {
184 identifier: builder.make_temporary(),
185 effect: None,
compiler/forget/crates/build-hir/src/builder.rs
+10 -8
@@ -1,10 +1,10 @@
1 use bumpalo::collections::Vec;
2 use estree::Identifier;
3 -use std::collections::HashSet;
3 +use std::{cell::RefCell, collections::HashSet, rc::Rc};
4
5 use hir::{
6 - BasicBlock, BlockId, BlockKind, Environment, GotoKind, Instruction, InstructionIdGenerator,
7 - InstructionValue, Place, Terminal, TerminalValue, Type, HIR,
6 + BasicBlock, BlockId, BlockKind, Environment, GotoKind, IdentifierData, Instruction,
7 + InstructionIdGenerator, InstructionValue, Place, Terminal, TerminalValue, Type, HIR,
8 };
9 use indexmap::IndexMap;
10
@@ -106,17 +106,19 @@ impl<'a> Builder<'a> {
106 pub(crate) fn make_temporary(&self) -> hir::Identifier<'a> {
107 hir::Identifier {
108 id: self.environment.next_identifier_id(),
109 - mutable_range: Default::default(),
109 name: None,
111 - scope: None,
112 - type_: Type::Var(self.environment.next_type_var_id()),
110 + data: Rc::new(RefCell::new(IdentifierData {
111 + mutable_range: Default::default(),
112 + scope: None,
113 + type_: Type::Var(self.environment.next_type_var_id()),
114 + })),
115 }
116 }
117
118 /// Resolves the target for the given break label (if present), or returns the default
119 /// break target given the current context. Returns a diagnostic if the label is
120 /// provided but cannot be resolved.
119 - pub(crate) fn resolve_break(&self, label: Option<Identifier>) -> Result<BlockId, Diagnostic> {
121 + pub(crate) fn resolve_break(&self, _label: Option<Identifier>) -> Result<BlockId, Diagnostic> {
122 todo!()
123 }
124
@@ -125,7 +127,7 @@ impl<'a> Builder<'a> {
127 /// provided but cannot be resolved.
128 pub(crate) fn resolve_continue(
129 &self,
128 - label: Option<Identifier>,
130 + _label: Option<Identifier>,
131 ) -> Result<BlockId, Diagnostic> {
132 todo!()
133 }
compiler/forget/crates/hir/src/environment.rs
+11 -1
@@ -2,7 +2,7 @@ use std::cell::Cell;
2
3 use bumpalo::Bump;
4
5 -use crate::{BlockId, Features, IdentifierId, Registry};
5 +use crate::{BlockId, Features, IdentifierId, Registry, TypeVarId};
6
7 /// Stores all the contextual information about the top-level React function being
8 /// compiled. Environments may not be reused between React functions, but *are*
@@ -26,6 +26,8 @@ pub struct Environment<'a> {
26
27 /// The next available identifier id
28 next_identifier_id: Cell<IdentifierId>,
29 +
30 + next_type_var_id: Cell<TypeVarId>,
31 }
32
33 impl<'a> Environment<'a> {
@@ -36,6 +38,7 @@ impl<'a> Environment<'a> {
38 registry,
39 next_block_id: Cell::new(BlockId(0)),
40 next_identifier_id: Cell::new(IdentifierId(0)),
41 + next_type_var_id: Cell::new(TypeVarId(0)),
42 }
43 }
44
@@ -57,4 +60,11 @@ impl<'a> Environment<'a> {
60 self.next_identifier_id.set(id.next());
61 id
62 }
63 +
64 + /// Get the next available type var
65 + pub fn next_type_var_id(&self) -> TypeVarId {
66 + let id = self.next_type_var_id.get();
67 + self.next_type_var_id.set(id.next());
68 + id
69 + }
70 }
compiler/forget/crates/hir/src/id_types.rs
+9
@@ -24,6 +24,15 @@ impl IdentifierId {
24 }
25 }
26
27 +#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Hash, Debug)]
28 +pub struct TypeVarId(pub(crate) u32);
29 +
30 +impl TypeVarId {
31 + pub(crate) fn next(self) -> Self {
32 + Self(self.0 + 1)
33 + }
34 +}
35 +
36 /// Used to globally order the instructions and terminals within the scope
37 /// of a given HIR value. Instructions and terminals are ordered using
38 /// reverse postorder iteration of block instructions and their terminals.
compiler/forget/crates/hir/src/instruction.rs
+23 -2
@@ -1,3 +1,5 @@
1 +use std::{cell::RefCell, rc::Rc};
2 +
3 use bumpalo::collections::{String, Vec};
4
5 use crate::{IdentifierId, InstructionId, ScopeId, Type};
@@ -20,7 +22,6 @@ pub enum InstructionValue<'a> {
22 DeclareContext(DeclareContext<'a>),
23 DeclareLocal(DeclareLocal<'a>),
24 // Destructure(Destructure<'a>),
23 - // Expression(Expression<'a>),
25 // Function(Function<'a>),
26 // JsxFragment(JsxFragment<'a>),
27 // JsxText(JsxText<'a>),
@@ -104,6 +105,7 @@ pub struct StoreLocal<'a> {
105 pub value: Place<'a>,
106 }
107
108 +#[derive(Clone)]
109 pub struct Place<'a> {
110 pub identifier: Identifier<'a>,
111 pub effect: Option<Effect>,
@@ -161,12 +163,16 @@ impl Effect {
163 }
164 }
165
166 +#[derive(Clone)]
167 pub struct Identifier<'a> {
168 /// Uniquely identifiers this identifier
169 pub id: IdentifierId,
167 -
170 pub name: Option<String<'a>>,
171
172 + pub data: Rc<RefCell<IdentifierData>>,
173 +}
174 +
175 +pub struct IdentifierData {
176 pub mutable_range: MutableRange,
177
178 pub scope: Option<ReactiveScope>,
@@ -187,6 +193,21 @@ pub struct MutableRange {
193 pub end: InstructionId,
194 }
195
196 +impl MutableRange {
197 + pub fn new() -> Self {
198 + Self {
199 + start: InstructionId(0),
200 + end: InstructionId(0),
201 + }
202 + }
203 +}
204 +
205 +impl Default for MutableRange {
206 + fn default() -> Self {
207 + Self::new()
208 + }
209 +}
210 +
211 pub struct ReactiveScope {
212 pub id: ScopeId,
213 pub range: MutableRange,
compiler/forget/crates/hir/src/terminal.rs
-2
@@ -1,5 +1,3 @@
1 -use std::iter::Successors;
2 -
1 use crate::{instruction::Place, BlockId, InstructionId};
2
3 /// Terminals represent statements or expressions that affect control flow,
compiler/forget/crates/hir/src/types.rs
+3 -3
@@ -1,9 +1,9 @@
1 -use crate::{FunctionId, ObjectId};
1 +use crate::{FunctionId, ObjectId, TypeVarId};
2
3 pub enum Type {
4 - Builtin(Box<BuiltinType>),
4 + Builtin(BuiltinType),
5 // Phi(Box<PhiType>),
6 - // Var(Box<TypeVar>),
6 + Var(TypeVarId),
7 // Poly(Box<PolyType>),
8 // Prop(Box<PropType>),
9 }