Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Error on empty function bodies #5519

Merged
merged 5 commits into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
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 Expand Up @@ -161,7 +165,7 @@
}
} else {
let name = self.interner.function_name(&function);
unreachable!("Non-builtin, lowlevel or oracle builtin fn '{name}'")

Check warning on line 168 in compiler/noirc_frontend/src/hir/comptime/interpreter.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (lowlevel)
}
}

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 @@ -194,7 +194,7 @@
/// Stores the [Location] of a [Type] reference
pub(crate) type_ref_locations: Vec<(Type, Location)>,

/// In Noir's metaprogramming, a noir type has the type `Type`. When these are spliced

Check warning on line 197 in compiler/noirc_frontend/src/node_interner.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (metaprogramming)
/// into `quoted` expressions, we preserve the original type by assigning it a unique id
/// and creating a `Token::QuotedType(id)` from this id. We cannot create a token holding
/// the actual type since types do not implement Send or Sync.
Expand Down Expand Up @@ -386,11 +386,6 @@
#[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 @@

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 @@
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
Loading