Skip to content

Commit

Permalink
fix: Error on empty function bodies (#5519)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves an issue mentioned in this PR:
#5511 And in @asterite's comptime
prefix operators PR.

## Summary\*

Previously, if a function was every (erroneously) allowed to be called
in a comptime context before it had been elaborated, we'd see just an
empty block as its body. I've changed this so we don't default function
bodies to an empty block anymore. Using an option we can now properly
error in this case as well.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
  • Loading branch information
jfecher and TomAFrench authored Jul 17, 2024
1 parent 43f5b8d commit 6a7f593
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 31 deletions.
6 changes: 5 additions & 1 deletion compiler/noirc_frontend/src/hir/comptime/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,11 @@ impl<'a> Interpreter<'a> {
self.define_pattern(parameter, typ, argument, arg_location)?;
}

let function_body = self.interner.function(&function).as_expr();
let function_body = self.interner.function(&function).try_as_expr().ok_or_else(|| {
let function = self.interner.function_name(&function).to_owned();
InterpreterError::NonComptimeFnCallInSameCrate { function, location }
})?;

let result = self.evaluate(function_body)?;

self.exit_function(previous_state);
Expand Down
9 changes: 5 additions & 4 deletions compiler/noirc_frontend/src/hir/comptime/scan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,12 @@ impl<'interner> Interpreter<'interner> {
return Ok(());
}

let function = self.interner.function(&function);
if let Some(function) = self.interner.function(&function).try_as_expr() {
let state = self.enter_function();
self.scan_expression(function)?;
self.exit_function(state);
}

let state = self.enter_function();
self.scan_expression(function.as_expr())?;
self.exit_function(state);
Ok(())
}

Expand Down
7 changes: 0 additions & 7 deletions compiler/noirc_frontend/src/hir_def/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,6 @@ pub enum HirExpression {
Error,
}

impl HirExpression {
/// Returns an empty block expression
pub const fn empty_block() -> HirExpression {
HirExpression::Block(HirBlockExpression { statements: vec![] })
}
}

/// Corresponds to a variable in the source code
#[derive(Debug, Clone)]
pub struct HirIdent {
Expand Down
18 changes: 11 additions & 7 deletions compiler/noirc_frontend/src/hir_def/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,26 +10,30 @@ use crate::macros_api::{BlockExpression, StructId};
use crate::node_interner::{ExprId, NodeInterner, TraitImplId};
use crate::{ResolvedGeneric, Type};

/// A Hir function is a block expression
/// with a list of statements
/// A Hir function is a block expression with a list of statements.
/// If the function has yet to be resolved, the body starts off empty (None).
#[derive(Debug, Clone)]
pub struct HirFunction(ExprId);
pub struct HirFunction(Option<ExprId>);

impl HirFunction {
pub fn empty() -> HirFunction {
HirFunction(ExprId::empty_block_id())
HirFunction(None)
}

pub const fn unchecked_from_expr(expr_id: ExprId) -> HirFunction {
HirFunction(expr_id)
HirFunction(Some(expr_id))
}

pub const fn as_expr(&self) -> ExprId {
pub fn as_expr(&self) -> ExprId {
self.0.expect("Function has yet to be elaborated, cannot get an ExprId of its body!")
}

pub fn try_as_expr(&self) -> Option<ExprId> {
self.0
}

pub fn block(&self, interner: &NodeInterner) -> HirBlockExpression {
match interner.expression(&self.0) {
match interner.expression(&self.as_expr()) {
HirExpression::Block(block_expr) => block_expr,
_ => unreachable!("ice: functions can only be block expressions"),
}
Expand Down
14 changes: 2 additions & 12 deletions compiler/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,11 +386,6 @@ impl StmtId {
#[derive(Debug, Eq, PartialEq, Hash, Copy, Clone, PartialOrd, Ord)]
pub struct ExprId(Index);

impl ExprId {
pub fn empty_block_id() -> ExprId {
ExprId(Index::unsafe_zeroed())
}
}
#[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)]
pub struct FuncId(Index);

Expand Down Expand Up @@ -558,7 +553,7 @@ pub struct QuotedTypeId(noirc_arena::Index);

impl Default for NodeInterner {
fn default() -> Self {
let mut interner = NodeInterner {
NodeInterner {
nodes: Arena::default(),
func_meta: HashMap::new(),
function_definition_ids: HashMap::new(),
Expand Down Expand Up @@ -598,12 +593,7 @@ impl Default for NodeInterner {
reference_graph: petgraph::graph::DiGraph::new(),
reference_graph_indices: HashMap::new(),
reference_modules: HashMap::new(),
};

// An empty block expression is used often, we add this into the `node` on startup
let expr_id = interner.push_expr(HirExpression::empty_block());
assert_eq!(expr_id, ExprId::empty_block_id());
interner
}
}
}

Expand Down

0 comments on commit 6a7f593

Please sign in to comment.