Skip to content

Commit

Permalink
fix: Error when a local function is called in a comptime context (#5334)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves #5333

## Summary\*

Issues an error when a non-comptime function in the same crate is called
from a comptime context:

```
error: `id` cannot be called in a `comptime` context here
  ┌─ /.../non_comptime_local_fn_call/src/main.nr:3:18
  │
3 │         let _a = id(3);
  │                  ----- This function must be `comptime` or in a separate crate to be called
  │
```

## Additional Context

This limitation is because all comptime items are elaborated before
non-comptime items within a crate.

## Documentation\*

Check one:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [x] **[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 Jun 26, 2024
1 parent 03e25b4 commit 7cd4a4d
Show file tree
Hide file tree
Showing 13 changed files with 87 additions and 34 deletions.
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 @@ pub enum InterpreterError {
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 @@ impl InterpreterError {
| 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 @@ -293,6 +295,12 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic {
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 noirc_errors::Location;
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 @@ pub struct Interpreter<'interner> {
/// up all currently visible definitions.
scopes: &'interner mut Vec<HashMap<DefinitionId, Value>>,

crate_id: CrateId,

in_loop: bool,
}

Expand All @@ -50,8 +53,9 @@ impl<'a> Interpreter<'a> {
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 @@ impl<'a> Interpreter<'a> {
});
}

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();
return Err(InterpreterError::NonComptimeFnCallInSameCrate { function, location });
}

if meta.kind != FunctionKind::Normal {
return self.call_builtin(function, arguments, location);
}
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

0 comments on commit 7cd4a4d

Please sign in to comment.