Merge pull request #965 from hash-org/tir-tweaks

TIR tweaks: Rename some variants, simplify loops, and make `Decl` specific to blocks

compiler/hash-intrinsics/src/intrinsics.rs

@@ -738,7 +738,7 @@ impl DefinedIntrinsics {
                 "print_fn_directives",
                 FnTy::builder().params(params).return_ty(ret).build(),
                 |_, args| {
-                    if let Term::FnRef(fn_def) = *args[1].value() {
+                    if let Term::Fn(fn_def) = *args[1].value() {
                         attr_store().map_with_default(fn_def.node_id_or_default(), |attrs| {
                             stream_less_writeln!("{:?}", attrs);
                         });

compiler/hash-lower/src/build/block.rs

@@ -12,7 +12,8 @@ use hash_storage::store::{statics::StoreId, TrivialSequenceStoreKey};
 use hash_tir::{
     context::{Context, ScopeKind},
     control::{LoopTerm, MatchTerm},
-    scopes::BlockTerm,
+    node::HasAstNodeId,
+    scopes::{BlockStatement, BlockTerm},
     terms::{Term, TermId},
 };
 
@@ -30,7 +31,7 @@ impl<'tcx> BodyBuilder<'tcx> {
 
         match *block_term.value() {
             Term::Block(ref body) => self.body_block_into_dest(place, block, body),
-            Term::Loop(LoopTerm { block: ref body }) => {
+            Term::Loop(LoopTerm { inner }) => {
                 // Begin the loop block by connecting the previous block
                 // and terminating it with a `goto` instruction to this block
                 let loop_body = self.control_flow_graph.start_new_block();
@@ -43,15 +44,15 @@ impl<'tcx> BodyBuilder<'tcx> {
                     // always going to be `()`
                     let tmp_place = this.make_tmp_unit();
 
-                    let body_block_end =
-                        unpack!(this.body_block_into_dest(tmp_place, loop_body, body));
+                    let body_block_end = unpack!(this.term_into_dest(tmp_place, loop_body, inner));
 
                     // In the situation that we have the final statement in the loop, this
                     // block should go back to the start of the loop...
                     if !this.control_flow_graph.is_terminated(body_block_end) {
                         this.control_flow_graph.goto(body_block_end, loop_body, span);
                     }
 
+                    this.reached_terminator = false;
                     next_block.unit()
                 })
             }
@@ -68,7 +69,7 @@ impl<'tcx> BodyBuilder<'tcx> {
         mut block: BasicBlock,
         body: &BlockTerm,
     ) -> BlockAnd<()> {
-        let BlockTerm { stack_id, statements, return_value } = body;
+        let BlockTerm { stack_id, statements, expr } = body;
 
         if self.reached_terminator {
             return block.unit();
@@ -84,11 +85,11 @@ impl<'tcx> BodyBuilder<'tcx> {
                     // We need to handle declarations here specifically, otherwise
                     // in order to not have to create a temporary for the declaration
                     // which doesn't make sense because we are just declaring a local(s)
-                    Term::Decl(decl) => {
-                        let span = this.span_of_term(statement);
+                    BlockStatement::Decl(decl) => {
+                        let span = statement.node_id_or_default();
                         unpack!(block = this.lower_declaration(block, &decl, span));
                     }
-                    _ => {
+                    BlockStatement::Expr(statement) => {
                         // @@Investigate: do we need to deal with the temporary here?
                         unpack!(
                             block = this.term_into_temp(block, statement, Mutability::Immutable)
@@ -100,7 +101,7 @@ impl<'tcx> BodyBuilder<'tcx> {
             // If this block has an expression, we need to deal with it since
             // it might change the destination of this block.
             if !this.reached_terminator {
-                unpack!(block = this.term_into_dest(place, block, *return_value));
+                unpack!(block = this.term_into_dest(place, block, *expr));
             }
         });
 

compiler/hash-lower/src/build/category.rs

@@ -55,12 +55,11 @@ impl Category {
             | Term::LoopControl(..)
             | Term::Block(_)
             | Term::Ctor(_)
-            | Term::FnRef(_)
+            | Term::Fn(_)
             | Term::Match(..)
-            | Term::FnCall(..) => Category::RValue(RValueKind::Into),
+            | Term::Call(..) => Category::RValue(RValueKind::Into),
 
             Term::Tuple(_)
-            | Term::Decl(_)
             | Term::Assign(_)
             | Term::Array(_)
             | Term::Cast(_)

compiler/hash-lower/src/build/into.rs

@@ -23,7 +23,7 @@ use hash_tir::{
     control::{LoopControlTerm, ReturnTerm},
     data::CtorTerm,
     environment::env::AccessToEnv,
-    fns::FnCallTerm,
+    fns::CallTerm,
     node::NodesId,
     params::ParamIndex,
     refs::{self, RefTerm},
@@ -129,7 +129,7 @@ impl<'tcx> BodyBuilder<'tcx> {
                     self.constructor_into_dest(destination, block, ctor, adt, span)
                 }
             }
-            Term::FnCall(ref fn_term @ FnCallTerm { subject, args, .. }) => {
+            Term::Call(ref fn_term @ CallTerm { subject, args, .. }) => {
                 match self.classify_fn_call_term(fn_term) {
                     FnCallTermKind::Call(_) => {
                         // Get the type of the function into or to to get the
@@ -282,10 +282,6 @@ impl<'tcx> BodyBuilder<'tcx> {
                 self.control_flow_graph.goto(block, return_block, span);
                 self.control_flow_graph.start_new_block().unit()
             }
-            // For declarations, we have to perform some bookkeeping in regards
-            // to locals..., but this expression should never return any value
-            // so we should just return a unit block here
-            Term::Decl(ref decl) => self.lower_declaration(block, decl, span),
             Term::Assign(assign_term) => {
                 // Deal with the actual assignment
                 block = unpack!(self.lower_assign_term(block, assign_term, span));
@@ -332,7 +328,7 @@ impl<'tcx> BodyBuilder<'tcx> {
             | Ty::RefTy(_)
             | Ty::Universe
             | Term::Hole(_)
-            | Term::FnRef(_) => block.unit(),
+            | Term::Fn(_) => block.unit(),
         };
 
         block_and

compiler/hash-lower/src/build/matches/declarations.rs

@@ -14,7 +14,7 @@ use hash_tir::{
     data::CtorPat,
     node::NodesId,
     pats::{Pat, PatId},
-    scopes::{BindingPat, DeclTerm},
+    scopes::{BindingPat, Decl},
     symbols::SymbolId,
     terms::TermId,
     tuples::TuplePat,
@@ -45,7 +45,7 @@ impl<'tcx> BodyBuilder<'tcx> {
     pub(crate) fn lower_declaration(
         &mut self,
         mut block: BasicBlock,
-        decl: &DeclTerm,
+        decl: &Decl,
         decl_origin: AstNodeId,
     ) -> BlockAnd<()> {
         if let Some(value) = &decl.value {

compiler/hash-lower/src/build/matches/mod.rs

@@ -107,7 +107,7 @@ impl<'tcx> BodyBuilder<'tcx> {
 
         // If this is a `&&`, we can create a `then-else` block sequence
         // that respects the short-circuiting behaviour of `&&`.
-        if let Term::FnCall(ref fn_call) = *expr.value() {
+        if let Term::Call(ref fn_call) = *expr.value() {
             if let FnCallTermKind::LogicalBinOp(LogicalBinOp::And, lhs, rhs) =
                 self.classify_fn_call_term(fn_call)
             {

compiler/hash-lower/src/build/mod.rs

@@ -172,9 +172,12 @@ pub(crate) struct BodyBuilder<'tcx> {
     /// after a block terminator.
     loop_block_info: Option<LoopBlockInfo>,
 
-    /// If the current [terms::BlockTerm] has reached a terminating statement,
-    /// i.e. a statement that is typed as `!`. Examples of such statements
-    /// are `return`, `break`, `continue`, etc.
+    /// If the lowerer has reached a terminating statement within some block,
+    /// meaning that further statements do not require to be lowered.
+    ///
+    /// A statement that is typed as `!`. Examples of such statements
+    /// are `return`, `break`, `continue`, or expressions that are of type
+    /// `!`.
     reached_terminator: bool,
 
     /// A temporary [Place] that is used to throw away results from expressions

compiler/hash-lower/src/build/place.rs

@@ -144,15 +144,14 @@ impl<'tcx> BodyBuilder<'tcx> {
             Term::Tuple(_)
             | Term::Lit(_)
             | Term::Array(_)
-            | Term::FnCall(_)
+            | Term::Call(_)
             | Term::Ctor(_)
-            | Term::FnRef(_)
+            | Term::Fn(_)
             | Term::Block(_)
             | Term::Loop(_)
             | Term::LoopControl(_)
             | Term::Match(_)
             | Term::Return(_)
-            | Term::Decl(_)
             | Term::Assign(_)
             | Term::Unsafe(_)
             | Term::Cast(_)

compiler/hash-lower/src/build/rvalue.rs

@@ -38,7 +38,7 @@ impl<'tcx> BodyBuilder<'tcx> {
                 let value = self.as_constant(lit).into();
                 block.and(value)
             }
-            ref fn_call_term @ Term::FnCall(fn_call) => {
+            ref fn_call_term @ Term::Call(fn_call) => {
                 match self.classify_fn_call_term(&fn_call) {
                     FnCallTermKind::BinaryOp(op, lhs, rhs) => {
                         let lhs = unpack!(block = self.as_operand(block, lhs, Mutability::Mutable));
@@ -113,14 +113,13 @@ impl<'tcx> BodyBuilder<'tcx> {
             ref term @ (Term::Array(_)
             | Term::Tuple(_)
             | Term::Ctor(_)
-            | Term::FnRef(_)
+            | Term::Fn(_)
             | Term::Block(_)
             | Term::Var(_)
             | Term::Loop(_)
             | Term::LoopControl(_)
             | Term::Match(_)
             | Term::Return(_)
-            | Term::Decl(_)
             | Term::Assign(_)
             | Term::Unsafe(_)
             | Term::Access(_)
@@ -172,7 +171,7 @@ impl<'tcx> BodyBuilder<'tcx> {
 
         // If the item is a reference to a function, i.e. the subject of a call, then
         // we emit a constant that refers to the function.
-        if let Term::FnRef(def_id) = *term {
+        if let Term::Fn(def_id) = *term {
             let ty_id = self.ty_id_from_tir_fn_def(def_id);
 
             // If this is a function type, we emit a ZST to represent the operand

compiler/hash-lower/src/build/ty.rs

@@ -20,7 +20,7 @@ use hash_tir::{
     atom_info::ItemInAtomInfo,
     data::DataTy,
     environment::env::AccessToEnv,
-    fns::{FnCallTerm, FnDefId},
+    fns::{CallTerm, FnDefId},
     lits::{Lit, LitPat},
     pats::PatId,
     terms::{Term, TermId, TyId},
@@ -37,7 +37,7 @@ use super::BodyBuilder;
 pub enum FnCallTermKind {
     /// A function call, the term doesn't change and should just be
     /// handled as a function call.
-    Call(FnCallTerm),
+    Call(CallTerm),
 
     /// A cast intrinsic operation, we perform a cast from the type of the
     /// first term into the desired second [IrTyId].
@@ -90,11 +90,11 @@ impl<'tcx> BodyBuilder<'tcx> {
 
     /// Function which is used to classify a [FnCallTerm] into a
     /// [FnCallTermKind].
-    pub(crate) fn classify_fn_call_term(&self, term: &FnCallTerm) -> FnCallTermKind {
-        let FnCallTerm { subject, args, .. } = term;
+    pub(crate) fn classify_fn_call_term(&self, term: &CallTerm) -> FnCallTermKind {
+        let CallTerm { subject, args, .. } = term;
 
         match *subject.value() {
-            Term::FnRef(fn_def) => {
+            Term::Fn(fn_def) => {
                 // Check if the fn_def is a `un_op` intrinsic
                 if fn_def == self.intrinsics().un_op() {
                     let (op, subject) =

compiler/hash-semantics/src/environment/ast_info.rs

@@ -3,7 +3,7 @@ use std::{collections::HashMap, fmt::Debug, hash::Hash};
 use hash_ast::ast::AstNodeId;
 use hash_tir::{
     args::{ArgId, ArgsSeqId, PatArgId, PatArgsSeqId},
-    context::Decl,
+    context::ContextMember,
     data::{CtorDefId, CtorDefsSeqId, DataDefId},
     fns::FnDefId,
     mods::{ModDefId, ModMemberId, ModMembersSeqId},
@@ -161,7 +161,7 @@ ast_info! {
     fn_defs: AstMap<FnDefId>,
 
     stacks: AstMap<StackId>,
-    stack_members: AstMap<Decl>,
+    stack_members: AstMap<ContextMember>,
 
     terms: AstMap<TermId>,
     tys: AstMap<TyId>,

compiler/hash-semantics/src/passes/discovery/defs.rs

@@ -9,7 +9,7 @@ use hash_storage::store::{
     DefaultPartialStore, PartialStore, SequenceStoreKey, StoreKey,
 };
 use hash_tir::{
-    context::Decl,
+    context::ContextMember,
     data::{CtorDef, CtorDefData, CtorDefId, DataDefCtors, DataDefId},
     environment::env::AccessToEnv,
     fns::FnDefId,
@@ -75,7 +75,7 @@ pub(super) enum ItemId {
 /// contain local definitions.
 #[derive(Debug, Copy, Clone, From)]
 enum StackMemberOrModMember {
-    StackMember(Decl),
+    StackMember(ContextMember),
     ModMember(ModMember),
 }
 
@@ -496,14 +496,15 @@ impl<'tc> DiscoveryPass<'tc> {
     /// resolved at a later stage.
     pub(super) fn add_stack_members_in_pat_to_buf(
         node: AstNodeRef<ast::Pat>,
-        buf: &mut SmallVec<[(AstNodeId, Decl); 3]>,
+        buf: &mut SmallVec<[(AstNodeId, ContextMember); 3]>,
     ) {
         let register_spread_pat =
-            |spread: &AstNode<ast::SpreadPat>, buf: &mut SmallVec<[(AstNodeId, Decl); 3]>| {
+            |spread: &AstNode<ast::SpreadPat>,
+             buf: &mut SmallVec<[(AstNodeId, ContextMember); 3]>| {
                 if let Some(name) = &spread.name {
                     buf.push((
                         name.id(),
-                        Decl {
+                        ContextMember {
                             name: SymbolId::from_name(name.ident, NodeOrigin::Given(name.id())),
                             ty: None,
                             value: None,
@@ -516,7 +517,7 @@ impl<'tc> DiscoveryPass<'tc> {
             ast::Pat::Binding(binding) => {
                 buf.push((
                     node.id(),
-                    Decl {
+                    ContextMember {
                         name: SymbolId::from_name(
                             binding.name.ident,
                             NodeOrigin::Given(binding.name.id()),
@@ -569,7 +570,7 @@ impl<'tc> DiscoveryPass<'tc> {
             }
             ast::Pat::Wild(_) => buf.push((
                 node.id(),
-                Decl {
+                ContextMember {
                     name: SymbolId::fresh(NodeOrigin::Given(node.id())),
                     // is_mutable: false,
                     ty: None,
@@ -607,8 +608,10 @@ impl<'tc> DiscoveryPass<'tc> {
                 (Some(declaration_name), ast::Pat::Binding(binding_pat))
                     if declaration_name.borrow().name == Some(binding_pat.name.ident) =>
                 {
-                    found_members
-                        .push((node.id(), Decl { name: declaration_name, ty: None, value: None }))
+                    found_members.push((
+                        node.id(),
+                        ContextMember { name: declaration_name, ty: None, value: None },
+                    ))
                 }
                 _ => Self::add_stack_members_in_pat_to_buf(node, &mut found_members),
             }

compiler/hash-semantics/src/passes/evaluation/mod.rs

@@ -10,7 +10,7 @@ use hash_storage::store::statics::SequenceStoreValue;
 use hash_tir::{
     args::Arg,
     environment::env::AccessToEnv,
-    fns::FnCallTerm,
+    fns::CallTerm,
     node::{Node, NodeId},
     terms::{Term, TermId},
     utils::common::dump_tir,
@@ -54,7 +54,7 @@ impl EvaluationPass<'_> {
                 match def {
                     Some(def) => {
                         let call_term = Term::from(
-                            FnCallTerm {
+                            CallTerm {
                                 subject: Term::from(def, def.origin()),
                                 implicit: false,
                                 args: Node::create_at(Node::<Arg>::empty_seq(), def.origin()),