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 when a local function is called in a comptime context #5334

Merged
merged 5 commits into from
Jun 26, 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: 4 additions & 2 deletions compiler/noirc_frontend/src/elaborator/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,8 @@ impl<'context> Elaborator<'context> {

fn elaborate_comptime_block(&mut self, block: BlockExpression, span: Span) -> (ExprId, Type) {
let (block, _typ) = self.elaborate_block_expression(block);
let mut interpreter = Interpreter::new(self.interner, &mut self.comptime_scopes);
let mut interpreter =
Interpreter::new(self.interner, &mut self.comptime_scopes, self.crate_id);
let value = interpreter.evaluate_block(block);
self.inline_comptime_value(value, span)
}
Expand Down Expand Up @@ -737,7 +738,8 @@ impl<'context> Elaborator<'context> {
}
};

let mut interpreter = Interpreter::new(self.interner, &mut self.comptime_scopes);
let mut interpreter =
Interpreter::new(self.interner, &mut self.comptime_scopes, self.crate_id);

let mut comptime_args = Vec::new();
let mut errors = Vec::new();
Expand Down
11 changes: 8 additions & 3 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,7 @@ impl<'context> Elaborator<'context> {
is_entry_point,
is_trait_function,
has_inline_attribute,
source_crate: self.crate_id,
function_body: FunctionBody::Unresolved(func.kind, body, func.def.span),
};

Expand Down Expand Up @@ -1237,8 +1238,11 @@ impl<'context> Elaborator<'context> {
let definition = self.interner.definition(id);
if let DefinitionKind::Function(function) = &definition.kind {
let function = *function;
let mut interpreter =
Interpreter::new(self.interner, &mut self.comptime_scopes);
let mut interpreter = Interpreter::new(
self.interner,
&mut self.comptime_scopes,
self.crate_id,
);

let location = Location::new(span, self.file);
let arguments = vec![(Value::TypeDefinition(struct_id), location)];
Expand Down Expand Up @@ -1327,7 +1331,8 @@ impl<'context> Elaborator<'context> {
let definition_id = global.definition_id;
let location = global.location;

let mut interpreter = Interpreter::new(self.interner, &mut self.comptime_scopes);
let mut interpreter =
Interpreter::new(self.interner, &mut self.comptime_scopes, self.crate_id);

if let Err(error) = interpreter.evaluate_let(let_statement) {
self.errors.push(error.into_compilation_error_pair());
Expand Down
3 changes: 2 additions & 1 deletion compiler/noirc_frontend/src/elaborator/statements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,8 @@ impl<'context> Elaborator<'context> {
fn elaborate_comptime_statement(&mut self, statement: Statement) -> (HirStatement, Type) {
let span = statement.span;
let (hir_statement, _typ) = self.elaborate_statement(statement);
let mut interpreter = Interpreter::new(self.interner, &mut self.comptime_scopes);
let mut interpreter =
Interpreter::new(self.interner, &mut self.comptime_scopes, self.crate_id);
let value = interpreter.evaluate_statement(hir_statement);
let (expr, typ) = self.inline_comptime_value(value, span);
(HirStatement::Expression(expr), typ)
Expand Down
8 changes: 8 additions & 0 deletions compiler/noirc_frontend/src/hir/comptime/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
CannotInlineMacro { value: Value, location: Location },
UnquoteFoundDuringEvaluation { location: Location },
FailedToParseMacro { error: ParserError, tokens: Rc<Tokens>, rule: &'static str, file: FileId },
NonComptimeFnCallInSameCrate { function: String, location: Location },

Unimplemented { item: String, location: Location },

Expand Down Expand Up @@ -101,6 +102,7 @@
| InterpreterError::NonStructInConstructor { location, .. }
| InterpreterError::CannotInlineMacro { location, .. }
| InterpreterError::UnquoteFoundDuringEvaluation { location, .. }
| InterpreterError::NonComptimeFnCallInSameCrate { location, .. }
| InterpreterError::Unimplemented { location, .. }
| InterpreterError::BreakNotInLoop { location, .. }
| InterpreterError::ContinueNotInLoop { location, .. } => *location,
Expand Down Expand Up @@ -272,7 +274,7 @@
let message = format!("Failed to parse macro's token stream into {rule}");
let tokens = vecmap(&tokens.0, ToString::to_string).join(" ");

// 10 is an aribtrary number of tokens here chosen to fit roughly onto one line

Check warning on line 277 in compiler/noirc_frontend/src/hir/comptime/errors.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (aribtrary)
let token_stream = if tokens.len() > 10 {
format!("The resulting token stream was: {tokens}")
} else {
Expand All @@ -293,6 +295,12 @@
diagnostic.add_note(push_the_problem_on_the_library_author);
diagnostic
}
InterpreterError::NonComptimeFnCallInSameCrate { function, location } => {
let msg = format!("`{function}` cannot be called in a `comptime` context here");
let secondary =
"This function must be `comptime` or in a separate crate to be called".into();
CustomDiagnostic::simple_error(msg, secondary, location.span)
}
InterpreterError::Unimplemented { item, location } => {
let msg = format!("{item} is currently unimplemented");
CustomDiagnostic::simple_error(msg, String::new(), location.span)
Expand Down
14 changes: 13 additions & 1 deletion compiler/noirc_frontend/src/hir/comptime/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use rustc_hash::FxHashMap as HashMap;

use crate::ast::{BinaryOpKind, FunctionKind, IntegerBitSize, Signedness};
use crate::graph::CrateId;
use crate::token::Tokens;
use crate::{
hir_def::{
Expand Down Expand Up @@ -42,6 +43,8 @@
/// up all currently visible definitions.
scopes: &'interner mut Vec<HashMap<DefinitionId, Value>>,

crate_id: CrateId,

in_loop: bool,
}

Expand All @@ -50,8 +53,9 @@
pub(crate) fn new(
interner: &'a mut NodeInterner,
scopes: &'a mut Vec<HashMap<DefinitionId, Value>>,
crate_id: CrateId,
) -> Self {
Self { interner, scopes, in_loop: false }
Self { interner, scopes, crate_id, in_loop: false }
}

pub(crate) fn call_function(
Expand All @@ -69,6 +73,14 @@
});
}

let is_comptime = self.interner.function_modifiers(&function).is_comptime;
if !is_comptime && meta.source_crate == self.crate_id {
// Calling non-comptime functions from within the current crate is restricted
// as non-comptime items will have not been elaborated yet.
let function = self.interner.function_name(&function).to_owned();
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
return Err(InterpreterError::NonComptimeFnCallInSameCrate { function, location });
}

if meta.kind != FunctionKind::Normal {
return self.call_builtin(function, arguments, location);
}
Expand Down Expand Up @@ -112,7 +124,7 @@
}
} else {
let name = self.interner.function_name(&function);
unreachable!("Non-builtin, lowlevel or oracle builtin fn '{name}'")

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

View workflow job for this annotation

GitHub Actions / Code

Unknown word (lowlevel)
}
}

Expand Down
33 changes: 17 additions & 16 deletions compiler/noirc_frontend/src/hir/comptime/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@ use noirc_errors::Location;
use super::errors::InterpreterError;
use super::interpreter::Interpreter;
use super::value::Value;
use crate::graph::CrateId;
use crate::hir::type_check::test::type_check_src_code;

fn interpret_helper(src: &str, func_namespace: Vec<String>) -> Result<Value, InterpreterError> {
let (mut interner, main_id) = type_check_src_code(src, func_namespace);
let mut scopes = vec![HashMap::default()];
let mut interpreter = Interpreter::new(&mut interner, &mut scopes);
let mut interpreter = Interpreter::new(&mut interner, &mut scopes, CrateId::Root(0));

let no_location = Location::dummy();
interpreter.call_function(main_id, Vec::new(), no_location)
Expand All @@ -30,14 +31,14 @@ fn interpret_expect_error(src: &str, func_namespace: Vec<String>) -> Interpreter

#[test]
fn interpreter_works() {
let program = "fn main() -> pub Field { 3 }";
let program = "comptime fn main() -> pub Field { 3 }";
let result = interpret(program, vec!["main".into()]);
assert_eq!(result, Value::Field(3u128.into()));
}

#[test]
fn mutation_works() {
let program = "fn main() -> pub i8 {
let program = "comptime fn main() -> pub i8 {
let mut x = 3;
x = 4;
x
Expand All @@ -48,7 +49,7 @@ fn mutation_works() {

#[test]
fn mutating_references() {
let program = "fn main() -> pub i32 {
let program = "comptime fn main() -> pub i32 {
let x = &mut 3;
*x = 4;
*x
Expand All @@ -59,7 +60,7 @@ fn mutating_references() {

#[test]
fn mutating_mutable_references() {
let program = "fn main() -> pub i64 {
let program = "comptime fn main() -> pub i64 {
let mut x = &mut 3;
*x = 4;
*x
Expand All @@ -70,7 +71,7 @@ fn mutating_mutable_references() {

#[test]
fn mutating_arrays() {
let program = "fn main() -> pub u8 {
let program = "comptime fn main() -> pub u8 {
let mut a1 = [1, 2, 3, 4];
a1[1] = 22;
a1[1]
Expand All @@ -81,7 +82,7 @@ fn mutating_arrays() {

#[test]
fn mutate_in_new_scope() {
let program = "fn main() -> pub u8 {
let program = "comptime fn main() -> pub u8 {
let mut x = 0;
x += 1;
{
Expand All @@ -95,7 +96,7 @@ fn mutate_in_new_scope() {

#[test]
fn for_loop() {
let program = "fn main() -> pub u8 {
let program = "comptime fn main() -> pub u8 {
let mut x = 0;
for i in 0 .. 6 {
x += i;
Expand All @@ -108,7 +109,7 @@ fn for_loop() {

#[test]
fn for_loop_u16() {
let program = "fn main() -> pub u16 {
let program = "comptime fn main() -> pub u16 {
let mut x = 0;
for i in 0 .. 6 {
x += i;
Expand All @@ -121,7 +122,7 @@ fn for_loop_u16() {

#[test]
fn for_loop_with_break() {
let program = "unconstrained fn main() -> pub u32 {
let program = "unconstrained comptime fn main() -> pub u32 {
let mut x = 0;
for i in 0 .. 6 {
if i == 4 {
Expand All @@ -137,7 +138,7 @@ fn for_loop_with_break() {

#[test]
fn for_loop_with_continue() {
let program = "unconstrained fn main() -> pub u64 {
let program = "unconstrained comptime fn main() -> pub u64 {
let mut x = 0;
for i in 0 .. 6 {
if i == 4 {
Expand All @@ -153,7 +154,7 @@ fn for_loop_with_continue() {

#[test]
fn assert() {
let program = "fn main() {
let program = "comptime fn main() {
assert(1 == 1);
}";
let result = interpret(program, vec!["main".into()]);
Expand All @@ -162,7 +163,7 @@ fn assert() {

#[test]
fn assert_fail() {
let program = "fn main() {
let program = "comptime fn main() {
assert(1 == 2);
}";
let result = interpret_expect_error(program, vec!["main".into()]);
Expand All @@ -171,7 +172,7 @@ fn assert_fail() {

#[test]
fn lambda() {
let program = "fn main() -> pub u8 {
let program = "comptime fn main() -> pub u8 {
let f = |x: u8| x + 1;
f(1)
}";
Expand All @@ -182,11 +183,11 @@ fn lambda() {
#[test]
fn non_deterministic_recursion() {
let program = "
fn main() -> pub u64 {
comptime fn main() -> pub u64 {
fib(10)
}

fn fib(x: u64) -> u64 {
comptime fn fib(x: u64) -> u64 {
if x <= 1 {
x
} else {
Expand Down
6 changes: 3 additions & 3 deletions compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ impl DefCollector {
resolved_module.type_check(context);

if !cycles_present {
resolved_module.evaluate_comptime(&mut context.def_interner);
resolved_module.evaluate_comptime(&mut context.def_interner, crate_id);
}

resolved_module.errors
Expand Down Expand Up @@ -546,10 +546,10 @@ impl ResolvedModule {
}

/// Evaluate all `comptime` expressions in this module
fn evaluate_comptime(&mut self, interner: &mut NodeInterner) {
fn evaluate_comptime(&mut self, interner: &mut NodeInterner, crate_id: CrateId) {
if self.count_errors() == 0 {
let mut scopes = vec![HashMap::default()];
let mut interpreter = Interpreter::new(interner, &mut scopes);
let mut interpreter = Interpreter::new(interner, &mut scopes, crate_id);

for (_file, global) in &self.globals {
if let Err(error) = interpreter.scan_global(*global) {
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1114,6 +1114,7 @@ impl<'a> Resolver<'a> {
trait_constraints: self.resolve_trait_constraints(&func.def.where_clause),
is_entry_point: self.is_entry_point_function(func),
has_inline_attribute,
source_crate: self.path_resolver.module_id().krate,

// These fields are only used by the elaborator
all_generics: Vec::new(),
Expand Down
17 changes: 10 additions & 7 deletions compiler/noirc_frontend/src/hir/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,7 @@ pub mod test {
all_generics: Vec::new(),
parameter_idents: Vec::new(),
function_body: FunctionBody::Resolved,
source_crate: CrateId::dummy_id(),
};
interner.push_fn_meta(func_meta, func_id);

Expand Down Expand Up @@ -716,13 +717,15 @@ pub mod test {
let mut interner = NodeInterner::default();
interner.populate_dummy_operator_traits();

assert_eq!(
errors.len(),
0,
"expected 0 parser errors, but got {}, errors: {:?}",
errors.len(),
errors
);
if !errors.iter().all(|error| error.is_warning()) {
assert_eq!(
errors.len(),
0,
"expected 0 parser errors, but got {}, errors: {:?}",
errors.len(),
errors
);
}

let func_ids = btree_map(&func_namespace, |name| {
(name.to_string(), interner.push_test_function_definition(name.into()))
Expand Down
4 changes: 4 additions & 0 deletions compiler/noirc_frontend/src/hir_def/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use super::expr::{HirBlockExpression, HirExpression, HirIdent};
use super::stmt::HirPattern;
use super::traits::TraitConstraint;
use crate::ast::{FunctionKind, FunctionReturnType, Visibility};
use crate::graph::CrateId;
use crate::macros_api::BlockExpression;
use crate::node_interner::{ExprId, NodeInterner, TraitImplId};
use crate::{ResolvedGeneric, Type, TypeVariable};
Expand Down Expand Up @@ -145,6 +146,9 @@ pub struct FuncMeta {
pub has_inline_attribute: bool,

pub function_body: FunctionBody,

/// The crate this function was defined in
pub source_crate: CrateId,
}

#[derive(Debug, Clone)]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "non_comptime_local_fn_call"
type = "bin"
authors = [""]
compiler_version = ">=0.31.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
fn main() {
comptime {
let _a = id(3);
}
}

fn id(x: Field) -> Field {
x
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ comptime global FOO: Field = foo();
// Due to this function's mutability and branching, SSA currently fails
// to fold this function into a constant before the assert_constant check
// is evaluated before loop unrolling.
fn foo() -> Field {
comptime fn foo() -> Field {
let mut three = 3;
if three == 3 { 5 } else { 6 }
}
Expand Down
Loading