From 1d2e7ee74706b57fb293089671153bac7e57ec93 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 6 Jul 2023 09:24:45 +0900 Subject: [PATCH] [rust] Use Rc> for shared identifiers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- compiler/forget/crates/build-hir/src/build.rs | 8 +++--- .../forget/crates/build-hir/src/builder.rs | 18 +++++++------ compiler/forget/crates/hir/src/environment.rs | 12 ++++++++- compiler/forget/crates/hir/src/id_types.rs | 9 +++++++ compiler/forget/crates/hir/src/instruction.rs | 25 +++++++++++++++++-- compiler/forget/crates/hir/src/terminal.rs | 2 -- compiler/forget/crates/hir/src/types.rs | 6 ++--- 7 files changed, 59 insertions(+), 21 deletions(-) diff --git a/compiler/forget/crates/build-hir/src/build.rs b/compiler/forget/crates/build-hir/src/build.rs index 0da854c700..abff514eab 100644 --- a/compiler/forget/crates/build-hir/src/build.rs +++ b/compiler/forget/crates/build-hir/src/build.rs @@ -52,7 +52,7 @@ fn lower_statement<'a>( env: &'a Environment<'a>, builder: &mut Builder<'a>, stmt: Statement, - label: Option>, + _label: Option>, ) -> Result<(), Diagnostic> { match stmt { Statement::BlockStatement(stmt) => { @@ -97,8 +97,6 @@ fn lower_statement<'a>( ); } Statement::ExpressionStatement(stmt) => { - // TODO: port the logic for emitting an ExpressionStatement instr if the instr - // was a logical or conditional. is that even necessary anymore? lower_expression_to_temporary(env, builder, stmt.expression); } Statement::EmptyStatement(_) => { @@ -175,13 +173,13 @@ fn lower_value_to_temporary<'a>( return place; } let place = build_temporary_place(env, builder); - builder.push(todo!("clone `place`"), value); + builder.push(place.clone(), value); return place; } /// Constructs a temporary Identifier and Place wrapper, which can be used as an Instruction lvalue /// or other places where a temporary target is required -fn build_temporary_place<'a>(env: &'a Environment<'a>, builder: &mut Builder<'a>) -> Place<'a> { +fn build_temporary_place<'a>(_env: &'a Environment<'a>, builder: &mut Builder<'a>) -> Place<'a> { Place { identifier: builder.make_temporary(), effect: None, diff --git a/compiler/forget/crates/build-hir/src/builder.rs b/compiler/forget/crates/build-hir/src/builder.rs index 48daf934cd..ebb68a4c00 100644 --- a/compiler/forget/crates/build-hir/src/builder.rs +++ b/compiler/forget/crates/build-hir/src/builder.rs @@ -1,10 +1,10 @@ use bumpalo::collections::Vec; use estree::Identifier; -use std::collections::HashSet; +use std::{cell::RefCell, collections::HashSet, rc::Rc}; use hir::{ - BasicBlock, BlockId, BlockKind, Environment, GotoKind, Instruction, InstructionIdGenerator, - InstructionValue, Place, Terminal, TerminalValue, Type, HIR, + BasicBlock, BlockId, BlockKind, Environment, GotoKind, IdentifierData, Instruction, + InstructionIdGenerator, InstructionValue, Place, Terminal, TerminalValue, Type, HIR, }; use indexmap::IndexMap; @@ -106,17 +106,19 @@ impl<'a> Builder<'a> { pub(crate) fn make_temporary(&self) -> hir::Identifier<'a> { hir::Identifier { id: self.environment.next_identifier_id(), - mutable_range: Default::default(), name: None, - scope: None, - type_: Type::Var(self.environment.next_type_var_id()), + data: Rc::new(RefCell::new(IdentifierData { + mutable_range: Default::default(), + scope: None, + type_: Type::Var(self.environment.next_type_var_id()), + })), } } /// Resolves the target for the given break label (if present), or returns the default /// break target given the current context. Returns a diagnostic if the label is /// provided but cannot be resolved. - pub(crate) fn resolve_break(&self, label: Option) -> Result { + pub(crate) fn resolve_break(&self, _label: Option) -> Result { todo!() } @@ -125,7 +127,7 @@ impl<'a> Builder<'a> { /// provided but cannot be resolved. pub(crate) fn resolve_continue( &self, - label: Option, + _label: Option, ) -> Result { todo!() } diff --git a/compiler/forget/crates/hir/src/environment.rs b/compiler/forget/crates/hir/src/environment.rs index 16f07c4301..8452b20993 100644 --- a/compiler/forget/crates/hir/src/environment.rs +++ b/compiler/forget/crates/hir/src/environment.rs @@ -2,7 +2,7 @@ use std::cell::Cell; use bumpalo::Bump; -use crate::{BlockId, Features, IdentifierId, Registry}; +use crate::{BlockId, Features, IdentifierId, Registry, TypeVarId}; /// Stores all the contextual information about the top-level React function being /// compiled. Environments may not be reused between React functions, but *are* @@ -26,6 +26,8 @@ pub struct Environment<'a> { /// The next available identifier id next_identifier_id: Cell, + + next_type_var_id: Cell, } impl<'a> Environment<'a> { @@ -36,6 +38,7 @@ impl<'a> Environment<'a> { registry, next_block_id: Cell::new(BlockId(0)), next_identifier_id: Cell::new(IdentifierId(0)), + next_type_var_id: Cell::new(TypeVarId(0)), } } @@ -57,4 +60,11 @@ impl<'a> Environment<'a> { self.next_identifier_id.set(id.next()); id } + + /// Get the next available type var + pub fn next_type_var_id(&self) -> TypeVarId { + let id = self.next_type_var_id.get(); + self.next_type_var_id.set(id.next()); + id + } } diff --git a/compiler/forget/crates/hir/src/id_types.rs b/compiler/forget/crates/hir/src/id_types.rs index c02b60487f..172fe4e8af 100644 --- a/compiler/forget/crates/hir/src/id_types.rs +++ b/compiler/forget/crates/hir/src/id_types.rs @@ -24,6 +24,15 @@ impl IdentifierId { } } +#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Hash, Debug)] +pub struct TypeVarId(pub(crate) u32); + +impl TypeVarId { + pub(crate) fn next(self) -> Self { + Self(self.0 + 1) + } +} + /// Used to globally order the instructions and terminals within the scope /// of a given HIR value. Instructions and terminals are ordered using /// reverse postorder iteration of block instructions and their terminals. diff --git a/compiler/forget/crates/hir/src/instruction.rs b/compiler/forget/crates/hir/src/instruction.rs index a0e5859831..c1fd83af61 100644 --- a/compiler/forget/crates/hir/src/instruction.rs +++ b/compiler/forget/crates/hir/src/instruction.rs @@ -1,3 +1,5 @@ +use std::{cell::RefCell, rc::Rc}; + use bumpalo::collections::{String, Vec}; use crate::{IdentifierId, InstructionId, ScopeId, Type}; @@ -20,7 +22,6 @@ pub enum InstructionValue<'a> { DeclareContext(DeclareContext<'a>), DeclareLocal(DeclareLocal<'a>), // Destructure(Destructure<'a>), - // Expression(Expression<'a>), // Function(Function<'a>), // JsxFragment(JsxFragment<'a>), // JsxText(JsxText<'a>), @@ -104,6 +105,7 @@ pub struct StoreLocal<'a> { pub value: Place<'a>, } +#[derive(Clone)] pub struct Place<'a> { pub identifier: Identifier<'a>, pub effect: Option, @@ -161,12 +163,16 @@ impl Effect { } } +#[derive(Clone)] pub struct Identifier<'a> { /// Uniquely identifiers this identifier pub id: IdentifierId, - pub name: Option>, + pub data: Rc>, +} + +pub struct IdentifierData { pub mutable_range: MutableRange, pub scope: Option, @@ -187,6 +193,21 @@ pub struct MutableRange { pub end: InstructionId, } +impl MutableRange { + pub fn new() -> Self { + Self { + start: InstructionId(0), + end: InstructionId(0), + } + } +} + +impl Default for MutableRange { + fn default() -> Self { + Self::new() + } +} + pub struct ReactiveScope { pub id: ScopeId, pub range: MutableRange, diff --git a/compiler/forget/crates/hir/src/terminal.rs b/compiler/forget/crates/hir/src/terminal.rs index 467fe1af79..17bcd168d4 100644 --- a/compiler/forget/crates/hir/src/terminal.rs +++ b/compiler/forget/crates/hir/src/terminal.rs @@ -1,5 +1,3 @@ -use std::iter::Successors; - use crate::{instruction::Place, BlockId, InstructionId}; /// Terminals represent statements or expressions that affect control flow, diff --git a/compiler/forget/crates/hir/src/types.rs b/compiler/forget/crates/hir/src/types.rs index ca1eda92f7..a8d6846c25 100644 --- a/compiler/forget/crates/hir/src/types.rs +++ b/compiler/forget/crates/hir/src/types.rs @@ -1,9 +1,9 @@ -use crate::{FunctionId, ObjectId}; +use crate::{FunctionId, ObjectId, TypeVarId}; pub enum Type { - Builtin(Box), + Builtin(BuiltinType), // Phi(Box), - // Var(Box), + Var(TypeVarId), // Poly(Box), // Prop(Box), }