Skip to content

Commit

Permalink
fix: Error when quote is used in runtime code (#5978)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

Fixes a few small issues:
- Error when `quote` is used in runtime code
  - Had to update some stdlib code for this
- This could maybe be an issue in the future for something like `impl
Append for Quoted` which relies on this. These impl methods have to be
comptime now which we currently allow even though the parent trait says
nothing about it. At least with
#5976 we'd get an error if one of
these accidentally sneaks into runtime code.
- `self.in_comptime_context()` wasn't being tracked accurately in the
case of attributes and globals where the function context would only be
of length 2 due to there being no outer function. I changed the check to
make it more direct instead of relying on indirect changes.
- Comptime globals weren't evaluated in their own function context
before so we waited to resolve traits until the end of the program. This
could lead to trait errors when interpreting them.

## 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\*

- [ ] I have tested the changes locally.
- [ ] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
jfecher authored Sep 9, 2024
1 parent 2adc24b commit cc30d88
Show file tree
Hide file tree
Showing 18 changed files with 145 additions and 84 deletions.
52 changes: 42 additions & 10 deletions compiler/noirc_frontend/src/elaborator/comptime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,16 +159,18 @@ impl<'context> Elaborator<'context> {
generated_items: &mut CollectedItems,
) {
if let SecondaryAttribute::Custom(attribute) = attribute {
if let Err(error) = self.run_comptime_attribute_name_on_item(
&attribute.contents,
item.clone(),
span,
attribute.contents_span,
attribute_context,
generated_items,
) {
self.errors.push(error);
}
self.elaborate_in_comptime_context(|this| {
if let Err(error) = this.run_comptime_attribute_name_on_item(
&attribute.contents,
item.clone(),
span,
attribute.contents_span,
attribute_context,
generated_items,
) {
this.errors.push(error);
}
});
}
}

Expand Down Expand Up @@ -599,4 +601,34 @@ impl<'context> Elaborator<'context> {
}
}
}

/// Perform the given function in a comptime context.
/// This will set the `in_comptime_context` flag on `self` as well as
/// push a new function context to resolve any trait constraints early.
pub(super) fn elaborate_in_comptime_context<T>(&mut self, f: impl FnOnce(&mut Self) -> T) -> T {
let old_comptime_value = std::mem::replace(&mut self.in_comptime_context, true);
// We have to push a new FunctionContext so that we can resolve any constraints
// in this comptime block early before the function as a whole finishes elaborating.
// Otherwise the interpreter below may find expressions for which the underlying trait
// call is not yet solved for.
self.function_context.push(Default::default());

let result = f(self);

self.check_and_pop_function_context();
self.in_comptime_context = old_comptime_value;
result
}

/// True if we're currently within a `comptime` block, function, or global
pub(super) fn in_comptime_context(&self) -> bool {
self.in_comptime_context
|| match self.current_item {
Some(DependencyId::Function(id)) => {
self.interner.function_modifiers(&id).is_comptime
}
Some(DependencyId::Global(id)) => self.interner.get_global_definition(id).comptime,
_ => false,
}
}
}
31 changes: 19 additions & 12 deletions compiler/noirc_frontend/src/elaborator/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl<'context> Elaborator<'context> {
ExpressionKind::Tuple(tuple) => self.elaborate_tuple(tuple),
ExpressionKind::Lambda(lambda) => self.elaborate_lambda(*lambda),
ExpressionKind::Parenthesized(expr) => return self.elaborate_expression(*expr),
ExpressionKind::Quote(quote) => self.elaborate_quote(quote),
ExpressionKind::Quote(quote) => self.elaborate_quote(quote, expr.span),
ExpressionKind::Comptime(comptime, _) => {
return self.elaborate_comptime_block(comptime, expr.span)
}
Expand Down Expand Up @@ -307,7 +307,13 @@ impl<'context> Elaborator<'context> {
let mut arguments = Vec::with_capacity(call.arguments.len());
let args = vecmap(call.arguments, |arg| {
let span = arg.span;
let (arg, typ) = self.elaborate_expression(arg);

let (arg, typ) = if call.is_macro_call {
self.elaborate_in_comptime_context(|this| this.elaborate_expression(arg))
} else {
self.elaborate_expression(arg)
};

arguments.push(arg);
(typ, arg, span)
});
Expand Down Expand Up @@ -735,20 +741,21 @@ impl<'context> Elaborator<'context> {
(expr, Type::Function(arg_types, Box::new(body_type), Box::new(env_type), false))
}

fn elaborate_quote(&mut self, mut tokens: Tokens) -> (HirExpression, Type) {
fn elaborate_quote(&mut self, mut tokens: Tokens, span: Span) -> (HirExpression, Type) {
tokens = self.find_unquoted_exprs_tokens(tokens);
(HirExpression::Quote(tokens), Type::Quoted(QuotedType::Quoted))

if self.in_comptime_context() {
(HirExpression::Quote(tokens), Type::Quoted(QuotedType::Quoted))
} else {
self.push_err(ResolverError::QuoteInRuntimeCode { span });
(HirExpression::Error, Type::Quoted(QuotedType::Quoted))
}
}

fn elaborate_comptime_block(&mut self, block: BlockExpression, span: Span) -> (ExprId, Type) {
// We have to push a new FunctionContext so that we can resolve any constraints
// in this comptime block early before the function as a whole finishes elaborating.
// Otherwise the interpreter below may find expressions for which the underlying trait
// call is not yet solved for.
self.function_context.push(Default::default());
let (block, _typ) = self.elaborate_block_expression(block);

self.check_and_pop_function_context();
let (block, _typ) =
self.elaborate_in_comptime_context(|this| this.elaborate_block_expression(block));

let mut interpreter = self.setup_interpreter();
let value = interpreter.evaluate_block(block);
let (id, typ) = self.inline_comptime_value(value, span);
Expand Down
31 changes: 12 additions & 19 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ pub struct Elaborator<'context> {
/// Initially empty, it is set whenever a new top-level item is resolved.
local_module: LocalModuleId,

/// True if we're elaborating a comptime item such as a comptime function,
/// block, global, or attribute.
in_comptime_context: bool,

crate_id: CrateId,

/// The scope of --debug-comptime, or None if unset
Expand Down Expand Up @@ -215,6 +219,7 @@ impl<'context> Elaborator<'context> {
unresolved_globals: BTreeMap::new(),
current_trait: None,
interpreter_call_stack,
in_comptime_context: false,
}
}

Expand Down Expand Up @@ -1289,7 +1294,12 @@ impl<'context> Elaborator<'context> {

let comptime = let_stmt.comptime;

let (let_statement, _typ) = self.elaborate_let(let_stmt, Some(global_id));
let (let_statement, _typ) = if comptime {
self.elaborate_in_comptime_context(|this| this.elaborate_let(let_stmt, Some(global_id)))
} else {
self.elaborate_let(let_stmt, Some(global_id))
};

let statement_id = self.interner.get_global(global_id).let_statement;
self.interner.replace_statement(statement_id, let_statement);

Expand Down Expand Up @@ -1324,9 +1334,7 @@ impl<'context> Elaborator<'context> {
.lookup_id(definition_id, location)
.expect("The global should be defined since evaluate_let did not error");

self.debug_comptime(location, |interner| {
interner.get_global(global_id).let_statement.to_display_ast(interner).kind
});
self.debug_comptime(location, |interner| value.display(interner).to_string());

self.interner.get_global_mut(global_id).value = Some(value);
}
Expand Down Expand Up @@ -1423,21 +1431,6 @@ impl<'context> Elaborator<'context> {
}
}

/// True if we're currently within a `comptime` block, function, or global
fn in_comptime_context(&self) -> bool {
// The first context is the global context, followed by the function-specific context.
// Any context after that is a `comptime {}` block's.
if self.function_context.len() > 2 {
return true;
}

match self.current_item {
Some(DependencyId::Function(id)) => self.interner.function_modifiers(&id).is_comptime,
Some(DependencyId::Global(id)) => self.interner.get_global_definition(id).comptime,
_ => false,
}
}

/// True if we're currently within a constrained function.
/// Defaults to `true` if the current function is unknown.
fn in_constrained_function(&self) -> bool {
Expand Down
9 changes: 2 additions & 7 deletions compiler/noirc_frontend/src/elaborator/statements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,14 +441,9 @@ impl<'context> Elaborator<'context> {
}

fn elaborate_comptime_statement(&mut self, statement: Statement) -> (HirStatement, Type) {
// We have to push a new FunctionContext so that we can resolve any constraints
// in this comptime block early before the function as a whole finishes elaborating.
// Otherwise the interpreter below may find expressions for which the underlying trait
// call is not yet solved for.
self.function_context.push(Default::default());
let span = statement.span;
let (hir_statement, _typ) = self.elaborate_statement(statement);
self.check_and_pop_function_context();
let (hir_statement, _typ) =
self.elaborate_in_comptime_context(|this| this.elaborate_statement(statement));
let mut interpreter = self.setup_interpreter();
let value = interpreter.evaluate_statement(hir_statement);
let (expr, typ) = self.inline_comptime_value(value, span);
Expand Down
9 changes: 9 additions & 0 deletions compiler/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ pub enum ResolverError {
AssociatedConstantsMustBeNumeric { span: Span },
#[error("Overflow in `{lhs} {op} {rhs}`")]
OverflowInType { lhs: u32, op: crate::BinaryTypeOperator, rhs: u32, span: Span },
#[error("`quote` cannot be used in runtime code")]
QuoteInRuntimeCode { span: Span },
}

impl ResolverError {
Expand Down Expand Up @@ -504,6 +506,13 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
*span,
)
}
ResolverError::QuoteInRuntimeCode { span } => {
Diagnostic::simple_error(
"`quote` cannot be used in runtime code".to_string(),
"Wrap this in a `comptime` block or function to use it".to_string(),
*span,
)
},
}
}
}
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3373,7 +3373,7 @@ fn unquoted_integer_as_integer_token() {
#[attr]
pub fn foobar() {}
fn attr(_f: FunctionDefinition) -> Quoted {
comptime fn attr(_f: FunctionDefinition) -> Quoted {
let serialized_len = 1;
// We are testing that when we unquote $serialized_len, it's unquoted
// as the token `1` and not as something else that later won't be parsed correctly
Expand Down
4 changes: 2 additions & 2 deletions noir_stdlib/src/append.nr
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ impl<T> Append for [T] {
}

impl Append for Quoted {
fn empty() -> Self {
comptime fn empty() -> Self {
quote {}
}

fn append(self, other: Self) -> Self {
comptime fn append(self, other: Self) -> Self {
quote { $self $other }
}
}
42 changes: 21 additions & 21 deletions noir_stdlib/src/meta/expr.nr
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ impl Expr {
}

// docs:start:quoted
fn quoted(self) -> Quoted {
comptime fn quoted(self) -> Quoted {
// docs:end:quoted
quote { $self }
}
Expand Down Expand Up @@ -381,12 +381,12 @@ fn modify_expressions<Env>(exprs: [Expr], f: fn[Env](Expr) -> Option<Expr>) -> [
exprs.map(|expr: Expr| expr.modify(f))
}

fn new_array(exprs: [Expr]) -> Expr {
comptime fn new_array(exprs: [Expr]) -> Expr {
let exprs = join_expressions(exprs, quote { , });
quote { [$exprs]}.as_expr().unwrap()
}

fn new_assert(predicate: Expr, msg: Option<Expr>) -> Expr {
comptime fn new_assert(predicate: Expr, msg: Option<Expr>) -> Expr {
if msg.is_some() {
let msg = msg.unwrap();
quote { assert($predicate, $msg) }.as_expr().unwrap()
Expand All @@ -395,7 +395,7 @@ fn new_assert(predicate: Expr, msg: Option<Expr>) -> Expr {
}
}

fn new_assert_eq(lhs: Expr, rhs: Expr, msg: Option<Expr>) -> Expr {
comptime fn new_assert_eq(lhs: Expr, rhs: Expr, msg: Option<Expr>) -> Expr {
if msg.is_some() {
let msg = msg.unwrap();
quote { assert_eq($lhs, $rhs, $msg) }.as_expr().unwrap()
Expand All @@ -404,30 +404,30 @@ fn new_assert_eq(lhs: Expr, rhs: Expr, msg: Option<Expr>) -> Expr {
}
}

fn new_assign(lhs: Expr, rhs: Expr) -> Expr {
comptime fn new_assign(lhs: Expr, rhs: Expr) -> Expr {
quote { $lhs = $rhs }.as_expr().unwrap()
}

fn new_binary_op(lhs: Expr, op: BinaryOp, rhs: Expr) -> Expr {
comptime fn new_binary_op(lhs: Expr, op: BinaryOp, rhs: Expr) -> Expr {
let op = op.quoted();
quote { ($lhs) $op ($rhs) }.as_expr().unwrap()
}

fn new_block(exprs: [Expr]) -> Expr {
comptime fn new_block(exprs: [Expr]) -> Expr {
let exprs = join_expressions(exprs, quote { ; });
quote { { $exprs }}.as_expr().unwrap()
}

fn new_cast(expr: Expr, typ: UnresolvedType) -> Expr {
comptime fn new_cast(expr: Expr, typ: UnresolvedType) -> Expr {
quote { ($expr) as $typ }.as_expr().unwrap()
}

fn new_comptime(exprs: [Expr]) -> Expr {
comptime fn new_comptime(exprs: [Expr]) -> Expr {
let exprs = join_expressions(exprs, quote { ; });
quote { comptime { $exprs }}.as_expr().unwrap()
}

fn new_if(condition: Expr, consequence: Expr, alternative: Option<Expr>) -> Expr {
comptime fn new_if(condition: Expr, consequence: Expr, alternative: Option<Expr>) -> Expr {
if alternative.is_some() {
let alternative = alternative.unwrap();
quote { if $condition { $consequence } else { $alternative }}.as_expr().unwrap()
Expand All @@ -436,11 +436,11 @@ fn new_if(condition: Expr, consequence: Expr, alternative: Option<Expr>) -> Expr
}
}

fn new_index(object: Expr, index: Expr) -> Expr {
comptime fn new_index(object: Expr, index: Expr) -> Expr {
quote { $object[$index] }.as_expr().unwrap()
}

fn new_let(pattern: Expr, typ: Option<UnresolvedType>, expr: Expr) -> Expr {
comptime fn new_let(pattern: Expr, typ: Option<UnresolvedType>, expr: Expr) -> Expr {
if typ.is_some() {
let typ = typ.unwrap();
quote { let $pattern : $typ = $expr; }.as_expr().unwrap()
Expand All @@ -449,17 +449,17 @@ fn new_let(pattern: Expr, typ: Option<UnresolvedType>, expr: Expr) -> Expr {
}
}

fn new_member_access(object: Expr, name: Quoted) -> Expr {
comptime fn new_member_access(object: Expr, name: Quoted) -> Expr {
quote { $object.$name }.as_expr().unwrap()
}

fn new_function_call(function: Expr, arguments: [Expr]) -> Expr {
comptime fn new_function_call(function: Expr, arguments: [Expr]) -> Expr {
let arguments = join_expressions(arguments, quote { , });

quote { $function($arguments) }.as_expr().unwrap()
}

fn new_method_call(object: Expr, name: Quoted, generics: [UnresolvedType], arguments: [Expr]) -> Expr {
comptime fn new_method_call(object: Expr, name: Quoted, generics: [UnresolvedType], arguments: [Expr]) -> Expr {
let arguments = join_expressions(arguments, quote { , });

if generics.len() == 0 {
Expand All @@ -470,30 +470,30 @@ fn new_method_call(object: Expr, name: Quoted, generics: [UnresolvedType], argum
}
}

fn new_repeated_element_array(expr: Expr, length: Expr) -> Expr {
comptime fn new_repeated_element_array(expr: Expr, length: Expr) -> Expr {
quote { [$expr; $length] }.as_expr().unwrap()
}

fn new_repeated_element_slice(expr: Expr, length: Expr) -> Expr {
comptime fn new_repeated_element_slice(expr: Expr, length: Expr) -> Expr {
quote { &[$expr; $length] }.as_expr().unwrap()
}

fn new_slice(exprs: [Expr]) -> Expr {
comptime fn new_slice(exprs: [Expr]) -> Expr {
let exprs = join_expressions(exprs, quote { , });
quote { &[$exprs]}.as_expr().unwrap()
}

fn new_tuple(exprs: [Expr]) -> Expr {
comptime fn new_tuple(exprs: [Expr]) -> Expr {
let exprs = join_expressions(exprs, quote { , });
quote { ($exprs) }.as_expr().unwrap()
}

fn new_unary_op(op: UnaryOp, rhs: Expr) -> Expr {
comptime fn new_unary_op(op: UnaryOp, rhs: Expr) -> Expr {
let op = op.quoted();
quote { $op($rhs) }.as_expr().unwrap()
}

fn new_unsafe(exprs: [Expr]) -> Expr {
comptime fn new_unsafe(exprs: [Expr]) -> Expr {
let exprs = join_expressions(exprs, quote { ; });
quote { unsafe { $exprs }}.as_expr().unwrap()
}
Expand Down
Loading

0 comments on commit cc30d88

Please sign in to comment.