Merge pull request #462 from Pat-Lafon/noret_brilirs

Noret brilirs

Author: sampsyo

brilirs/src/basic_block.rs

@@ -162,6 +162,7 @@ impl BBFunction {
           },
         ) => {
           if curr_block.label.is_some() || blocks.is_empty() {
+            curr_block.positions.push(i.get_pos());
             curr_block
               .flat_instrs
               .push(FlatIR::new(i, func_map, &mut num_var_map, &label_map)?);
@@ -170,6 +171,7 @@ impl BBFunction {
           curr_block = BasicBlock::new();
         }
         bril_rs::Code::Instruction(code) => {
+          curr_block.positions.push(code.get_pos());
           curr_block
             .flat_instrs
             .push(FlatIR::new(code, func_map, &mut num_var_map, &label_map)?);

brilirs/src/check.rs

@@ -560,6 +560,8 @@ fn type_check_func(func: &Function, prog: &Program) -> Result<(), PositionalInte
       .map_err(|e| e.add_pos(func.pos.clone()));
   }
 
+  let mut has_return_type_should_have_return = func.return_type.is_none();
+
   let mut env: FxHashMap<&str, Type> =
     FxHashMap::with_capacity_and_hasher(20, fxhash::FxBuildHasher::default());
   func.args.iter().for_each(|a| {
@@ -598,21 +600,34 @@ fn type_check_func(func: &Function, prog: &Program) -> Result<(), PositionalInte
   func.instrs.iter().try_for_each(|i| match i {
     bril_rs::Code::Label { .. } => Ok(()),
     bril_rs::Code::Instruction(instr) => {
+      if matches!(
+        instr,
+        Instruction::Effect {
+          op: EffectOps::Return,
+          ..
+        }
+      ) {
+        has_return_type_should_have_return = true;
+      }
       type_check_instruction(instr, func, prog, &mut env).map_err(|e| e.add_pos(instr.get_pos()))
     }
   })?;
 
-  Ok(())
+  if has_return_type_should_have_return {
+    Ok(())
+  } else {
+    Err(InterpError::NonVoidFuncNoRet(func.return_type.clone().unwrap()).add_pos(func.pos.clone()))
+  }
 }
 
 /// Provides validation of Bril programs. This involves
 /// statically checking the types and number of arguments to Bril
 /// instructions.
 /// # Errors
 /// Will return an error if typechecking fails or if the input program is not well-formed.
-pub fn type_check(bbprog: &Program) -> Result<(), PositionalInterpError> {
-  bbprog
+pub fn type_check(prog: &Program) -> Result<(), PositionalInterpError> {
+  prog
     .functions
     .iter()
-    .try_for_each(|bbfunc| type_check_func(bbfunc, bbprog))
+    .try_for_each(|bbfunc| type_check_func(bbfunc, prog))
 }

brilirs/src/error.rs

@@ -1,6 +1,6 @@
 use std::fmt::Display;
 
-use bril_rs::{Position, conversion::PositionalConversionError};
+use bril_rs::{Position, Type, conversion::PositionalConversionError};
 use std::error::Error;
 use thiserror::Error;
 
@@ -27,6 +27,8 @@ pub enum InterpError {
   DuplicateLabel(String),
   #[error("Expected empty return for `{0}`, found value")]
   NonEmptyRetForFunc(String),
+  #[error("non-void function (type {0}) didn't return anything")]
+  NonVoidFuncNoRet(Type),
   #[error("cannot allocate `{0}` entries")]
   CannotAllocSize(i64),
   #[error("Tried to free illegal memory location base: `{0}`, offset: `{1}`. Offset must be 0.")]

brilirs/src/interp.rs

@@ -671,6 +671,22 @@ fn execute<'a, T: std::io::Write>(
     }
 
     if !jumped {
+      if let Some(ty) = &func.return_type {
+        return Err(
+          InterpError::NonVoidFuncNoRet(ty.clone()).add_pos(if curr_instrs.is_empty() {
+            // Ideally we use the last instruction in the block before the fatal
+            // (implicit) return... but if that block is empty, we should just
+            // point to the function itself
+            func.pos.clone()
+          } else {
+            curr_block
+              .positions
+              .get(curr_instrs.len() - 1)
+              .cloned()
+              .unwrap_or_default()
+          }),
+        );
+      }
       return Ok(None);
     }
   }