Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
db3343e
feat: add generic arithmetics (2nd version), add arith expr struct an…
michaeljklein May 20, 2024
8e1ace9
add error from other branch, pre-process generic arith relevant expre…
michaeljklein May 20, 2024
7a51a52
wip
michaeljklein May 22, 2024
7927fbf
remove debugging panic, add ArithConstraints to aztec_macros, add loc…
michaeljklein May 22, 2024
39433d2
add expected failure, update expected success test, use a ref cell in…
michaeljklein May 23, 2024
75f2673
wip, add other_locations method for arith constraint errors, put othe…
michaeljklein May 28, 2024
123ecdb
wip debugging: add errors for defining unsupported impl's on GenericA…
michaeljklein May 28, 2024
af0afbe
wip TypeVariableId's don't appear to be consistent w/ named generic T…
michaeljklein May 29, 2024
7ac0c42
got key test case working, using GenericIndex + offsets, keep Generic…
michaeljklein May 29, 2024
8b43b96
wip debugging new error, "Type annotations needed" on function, inter…
michaeljklein May 29, 2024
a5ba238
wip debugging 'Type annotations needed', added GenericArith case to t…
michaeljklein May 29, 2024
f80acf4
add location to errors, adding arith expr errors to 'annotation neede…
michaeljklein May 30, 2024
ddefa27
uncomment unit tests
michaeljklein May 30, 2024
3392d5a
Merge branch 'master' into michaeljklein/interned-arith-generics
michaeljklein May 30, 2024
eb24682
cargo fmt/clippy, cleanup pass
michaeljklein May 30, 2024
e6f039c
todo/cleanup pass
michaeljklein May 30, 2024
d5805ca
clippy pass
michaeljklein May 30, 2024
ac983c4
clippy pass, refactor copy/pasted code, replace redundant HashMap wit…
michaeljklein May 30, 2024
36918cd
cargo fmt
michaeljklein May 30, 2024
506442e
Merge branch 'master' into michaeljklein/interned-arith-generics
michaeljklein May 30, 2024
c859821
Merge branch 'master' into michaeljklein/interned-arith-generics
michaeljklein May 30, 2024
f360857
wip moving changes over to elaborator, renamed prepare_arith_expressi…
michaeljklein May 30, 2024
a762e9c
cleanup copy/pasted block, properly recurse in follow_bindings, repla…
michaeljklein May 30, 2024
a1f4aef
cargo clippy/fmt, move generic arith to own module, TODO's including …
michaeljklein May 30, 2024
4d9ccef
clippy/fmt, ignore debugger generic arithmetic test, add prevent_gene…
michaeljklein May 31, 2024
614c84b
partially revert commented out assertion to answer questions for PR
michaeljklein May 31, 2024
7ab7503
Merge branch 'master' into michaeljklein/interned-arith-generics
michaeljklein May 31, 2024
33f2204
cargo fmt
michaeljklein May 31, 2024
c44a7e6
Merge branch 'master' into michaeljklein/interned-arith-generics
michaeljklein May 31, 2024
03565dc
add backwards_tail and cons tests
michaeljklein May 31, 2024
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 8 additions & 2 deletions compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,16 @@ use acvm::{
FieldElement,
};

use noirc_errors::debug_info::{DebugFunctions, DebugInfo, DebugTypes, DebugVariables};
use noirc_errors::{
debug_info::{DebugFunctions, DebugInfo, DebugTypes, DebugVariables},
Location,
};

use noirc_frontend::ast::Visibility;
use noirc_frontend::{
hir_def::{function::FunctionSignature, types::Type as HirType},
monomorphization::ast::Program,
node_interner::NodeInterner,
};
use tracing::{span, Level};

Expand Down Expand Up @@ -278,7 +282,9 @@ fn split_public_and_private_inputs(
.0
.iter()
.map(|(_, typ, visibility)| {
let num_field_elements_needed = typ.field_count() as usize;
let dummy_interner = NodeInterner::default();
let num_field_elements_needed =
typ.field_count(&Location::dummy(), &dummy_interner) as usize;
let witnesses = input_witnesses[idx..idx + num_field_elements_needed].to_vec();
idx += num_field_elements_needed;
(visibility, witnesses)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ use std::rc::Rc;
use crate::ssa::ir::{types::Type, value::ValueId};
use acvm::FieldElement;
use fxhash::FxHashMap as HashMap;
use noirc_errors::Location;
use noirc_frontend::ast;
use noirc_frontend::hir_def::function::FunctionSignature;
use noirc_frontend::node_interner::NodeInterner;

use super::FunctionBuilder;

Expand Down Expand Up @@ -37,7 +39,8 @@ impl DataBusBuilder {
ast::Visibility::Public | ast::Visibility::Private => false,
ast::Visibility::DataBus => true,
};
let len = param.1.field_count() as usize;
let dummy_interner = NodeInterner::default();
let len = param.1.field_count(&Location::dummy(), &dummy_interner) as usize;
params_is_databus.extend(vec![is_databus; len]);
}
params_is_databus
Expand Down
5 changes: 5 additions & 0 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -801,6 +801,11 @@ impl<'context> Elaborator<'context> {
}
}
}
Type::GenericArith(_, generics) => {
for generic in generics {
Self::find_numeric_generics_in_type(generic, found);
}
}
Type::MutableReference(element) => Self::find_numeric_generics_in_type(element, found),
Type::String(length) => {
if let Type::NamedGeneric(type_variable, name) = length.as_ref() {
Expand Down
100 changes: 85 additions & 15 deletions compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::collections::HashSet;
use std::rc::Rc;

use acvm::acir::AcirField;
Expand Down Expand Up @@ -29,7 +30,10 @@ use crate::{
HirExpression, HirLiteral, HirStatement, Path, PathKind, SecondaryAttribute, Signedness,
UnaryOp, UnresolvedType, UnresolvedTypeData,
},
node_interner::{DefinitionKind, ExprId, GlobalId, TraitId, TraitImplKind, TraitMethodId},
node_interner::{
generic_arith::{ArithExpr, ArithOpKind},
DefinitionKind, ExprId, GlobalId, TraitId, TraitImplKind, TraitMethodId,
},
Generics, Type, TypeBinding, TypeVariable, TypeVariableKind,
};

Expand Down Expand Up @@ -272,32 +276,98 @@ impl<'context> Elaborator<'context> {
pub(super) fn convert_expression_type(&mut self, length: UnresolvedTypeExpression) -> Type {
match length {
UnresolvedTypeExpression::Variable(path) => {
self.lookup_generic_or_global_type(&path).unwrap_or_else(|| {
self.push_err(ResolverError::NoSuchNumericTypeVariable { path });
Type::Constant(0)
})
let var_or_constant =
self.lookup_generic_or_global_type(&path).unwrap_or_else(|| {
self.push_err(ResolverError::NoSuchNumericTypeVariable {
path: path.clone(),
});
Type::Constant(0)
});
if let Type::NamedGeneric(ref binding, ref name) = var_or_constant {
// we intern variables so that they can be resolved during trait resolution
let arith_expr =
ArithExpr::Variable(binding.clone(), name.clone(), Default::default());
let location = Location { span: path.span, file: self.file };
let _ = self.interner.push_arith_expression(arith_expr, location);
}
var_or_constant
}
UnresolvedTypeExpression::Constant(int, span) => {
// we intern constants so that they can be resolved during trait resolution
let arith_expr = ArithExpr::Constant(int);
let location = Location { span, file: self.file };
let _ = self.interner.push_arith_expression(arith_expr, location);
Type::Constant(int)
}
UnresolvedTypeExpression::Constant(int, _) => Type::Constant(int),
UnresolvedTypeExpression::BinaryOperation(lhs, op, rhs, _) => {
UnresolvedTypeExpression::BinaryOperation(lhs, op, rhs, op_span) => {
let (lhs_span, rhs_span) = (lhs.span(), rhs.span());
let lhs = self.convert_expression_type(*lhs);
let rhs = self.convert_expression_type(*rhs);

let span = if !matches!(lhs, Type::Constant(_)) { lhs_span } else { rhs_span };
let (lhs, lhs_generics) = self.convert_type_to_arith_expr(lhs, span);
let (rhs, rhs_generics) = self.convert_type_to_arith_expr(rhs, span);

match (lhs, rhs) {
(Type::Constant(lhs), Type::Constant(rhs)) => {
Type::Constant(op.function()(lhs, rhs))
(ArithExpr::Constant(lhs), ArithExpr::Constant(rhs)) => {
let int = op.function()(lhs, rhs);
let arith_expr = ArithExpr::Constant(int);
let op_location = Location { span: op_span, file: self.file };
let _ = self.interner.push_arith_expression(arith_expr, op_location);
assert!(lhs_generics.is_empty(), "constant generics expected to be empty");
assert!(rhs_generics.is_empty(), "constant generics expected to be empty");
Type::Constant(int)
}
(lhs, _) => {
let span =
if !matches!(lhs, Type::Constant(_)) { lhs_span } else { rhs_span };
self.push_err(ResolverError::InvalidArrayLengthExpr { span });
Type::Constant(0)

(lhs, rhs) => {
let kind =
ArithOpKind::from_binary_type_operator(op).unwrap_or_else(|| {
self.push_err(ResolverError::InvalidGenericArithOp {
span: op_span,
});
// return a valid ArithOpKind when erroring
ArithOpKind::Add
});

// offset GenericIndex's in rhs to prevent overlap
let rhs = rhs.offset_generic_indices(lhs_generics.len());

let arith_expr =
ArithExpr::Op { kind, lhs: Box::new(lhs), rhs: Box::new(rhs) };
let op_location = Location { span: op_span, file: self.file };
let new_id = self.interner.push_arith_expression(arith_expr, op_location);
let new_generics = lhs_generics
.into_iter()
.chain(rhs_generics)
.collect::<HashSet<_>>()
.into_iter()
.collect::<Vec<_>>();

Type::GenericArith(new_id, new_generics)
}
}
}
}
}

fn convert_type_to_arith_expr(&mut self, typ: Type, span: Span) -> (ArithExpr, Vec<Type>) {
match typ {
Type::Constant(value) => (ArithExpr::Constant(value), vec![]),
Type::NamedGeneric(typevar, name) => (
ArithExpr::Variable(typevar.clone(), name.clone(), Default::default()),
vec![Type::NamedGeneric(typevar, name)],
),
Type::GenericArith(arith_id, generics) => {
let (arith_expr, _location) = self.interner.get_arith_expression(arith_id);
(arith_expr.clone(), generics)
}
_ => {
self.push_err(ResolverError::InvalidArrayLengthExpr { span });
(ArithExpr::Constant(0), vec![])
}
}
}

// this resolves Self::some_static_method, inside an impl block (where we don't have a concrete self_type)
//
// Returns the trait method, trait constraint, and whether the impl is assumed to exist by a where clause or not
Expand Down Expand Up @@ -547,7 +617,7 @@ impl<'context> Elaborator<'context> {
make_error: impl FnOnce() -> TypeCheckError,
) {
let mut errors = Vec::new();
actual.unify(expected, &mut errors, make_error);
actual.unify(expected, &self.interner.arith_constraints, &mut errors, make_error);
self.errors.extend(errors.into_iter().map(|error| (error.into(), self.file)));
}

Expand Down
11 changes: 7 additions & 4 deletions compiler/noirc_frontend/src/hir/comptime/errors.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use crate::{hir::def_collector::dc_crate::CompilationError, Type};
use crate::{
hir::def_collector::dc_crate::CompilationError, node_interner::generic_arith::ArithExprError,
Type,
};
use acvm::{acir::AcirField, FieldElement};
use noirc_errors::{CustomDiagnostic, Location};

Expand All @@ -23,7 +26,7 @@ pub enum InterpreterError {
NonArrayIndexed { value: Value, location: Location },
NonIntegerUsedAsIndex { value: Value, location: Location },
NonIntegerIntegerLiteral { typ: Type, location: Location },
NonIntegerArrayLength { typ: Type, location: Location },
NonIntegerArrayLength { typ: Type, location: Location, arith_expr_error: ArithExprError },
NonNumericCasted { value: Value, location: Location },
IndexOutOfBounds { index: usize, length: usize, location: Location },
ExpectedStructToHaveField { value: Value, field_name: String, location: Location },
Expand Down Expand Up @@ -196,9 +199,9 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic {
let secondary = "This is likely a bug".into();
CustomDiagnostic::simple_error(msg, secondary, location.span)
}
InterpreterError::NonIntegerArrayLength { typ, location } => {
InterpreterError::NonIntegerArrayLength { typ, location, arith_expr_error } => {
let msg = format!("Non-integer array length: `{typ}`");
let secondary = "Array lengths must be integers".into();
let secondary = format!("Array lengths must be integers\n{}", arith_expr_error);
CustomDiagnostic::simple_error(msg, secondary, location.span)
}
InterpreterError::NonNumericCasted { value, location } => {
Expand Down
66 changes: 46 additions & 20 deletions compiler/noirc_frontend/src/hir/comptime/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ use crate::{
},
},
macros_api::{HirExpression, HirLiteral, HirStatement, NodeInterner},
node_interner::{DefinitionId, DefinitionKind, ExprId, FuncId, StmtId},
node_interner::{
generic_arith::ArithExprError, DefinitionId, DefinitionKind, ExprId, FuncId, StmtId,
},
Shared, Type, TypeBinding, TypeBindings, TypeVariableKind,
};

Expand Down Expand Up @@ -284,13 +286,16 @@ impl<'a> Interpreter<'a> {
Err(InterpreterError::NonComptimeVarReferenced { name, location })
}

fn type_check(&self, typ: &Type, value: &Value, location: Location) -> IResult<()> {
fn type_check(&mut self, typ: &Type, value: &Value, location: Location) -> IResult<()> {
let typ = typ.follow_bindings();
let value_type = value.get_type();

typ.try_unify(&value_type, &mut TypeBindings::new()).map_err(|_| {
InterpreterError::TypeMismatch { expected: typ, value: value.clone(), location }
})
typ.try_unify(&value_type, &mut TypeBindings::new(), &self.interner.arith_constraints)
.map_err(|_| InterpreterError::TypeMismatch {
expected: typ,
value: value.clone(),
location,
})
}

/// Evaluate an expression and return the result
Expand Down Expand Up @@ -347,17 +352,30 @@ impl<'a> Interpreter<'a> {
}
DefinitionKind::GenericType(type_variable) => {
let value = match &*type_variable.borrow() {
TypeBinding::Unbound(_) => None,
TypeBinding::Bound(binding) => binding.evaluate_to_u64(),
TypeBinding::Unbound(var_id) => Err(ArithExprError::UnboundVariable {
binding: type_variable.clone(),
name: format!("#type_variable_id_{}_{:?}", var_id, ident),
}),
TypeBinding::Bound(binding) => {
binding.evaluate_to_u64(&definition.location, self.interner)
}
};

if let Some(value) = value {
let typ = self.interner.id_type(id);
self.evaluate_integer((value as u128).into(), false, id)
} else {
let location = self.interner.expr_location(&id);
let typ = Type::TypeVariable(type_variable.clone(), TypeVariableKind::Normal);
Err(InterpreterError::NonIntegerArrayLength { typ, location })
match value {
Ok(value) => {
let typ = self.interner.id_type(id);
self.evaluate_integer((value as u128).into(), false, id)
}
Err(arith_expr_error) => {
let location = self.interner.expr_location(&id);
let typ =
Type::TypeVariable(type_variable.clone(), TypeVariableKind::Normal);
Err(InterpreterError::NonIntegerArrayLength {
typ,
location,
arith_expr_error,
})
}
}
}
}
Expand Down Expand Up @@ -500,13 +518,21 @@ impl<'a> Interpreter<'a> {
}
HirArrayLiteral::Repeated { repeated_element, length } => {
let element = self.evaluate(repeated_element)?;
let location = self.interner.expr_location(&id);

if let Some(length) = length.evaluate_to_u64() {
let elements = (0..length).map(|_| element.clone()).collect();
Ok(Value::Array(elements, typ))
} else {
let location = self.interner.expr_location(&id);
Err(InterpreterError::NonIntegerArrayLength { typ: length, location })
match length.evaluate_to_u64(&location, self.interner) {
Ok(length) => {
let elements = (0..length).map(|_| element.clone()).collect();
Ok(Value::Array(elements, typ))
}
Err(arith_expr_error) => {
let location = self.interner.expr_location(&id);
Err(InterpreterError::NonIntegerArrayLength {
typ: length,
location,
arith_expr_error,
})
}
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion compiler/noirc_frontend/src/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ pub mod type_check;
use crate::debug::DebugInstrumenter;
use crate::graph::{CrateGraph, CrateId};
use crate::hir_def::function::FuncMeta;
use crate::node_interner::{FuncId, NodeInterner, StructId};
use crate::node_interner::{ArithConstraints, FuncId, NodeInterner, StructId};
use crate::parser::ParserError;
use crate::ParsedModule;
use def_map::{Contract, CrateDefMap};
Expand Down Expand Up @@ -43,6 +43,8 @@ pub struct Context<'file_manager, 'parsed_files> {
// Same as the file manager, we take ownership of the parsed files in the WASM context.
// Parsed files is also read only.
pub parsed_files: Cow<'parsed_files, ParsedFiles>,

pub arith_constraints: ArithConstraints,
}

#[derive(Debug, Copy, Clone)]
Expand All @@ -62,6 +64,7 @@ impl Context<'_, '_> {
file_manager: Cow::Owned(file_manager),
debug_instrumenter: DebugInstrumenter::default(),
parsed_files: Cow::Owned(parsed_files),
arith_constraints: Vec::new().into(),
}
}

Expand All @@ -77,6 +80,7 @@ impl Context<'_, '_> {
file_manager: Cow::Borrowed(file_manager),
debug_instrumenter: DebugInstrumenter::default(),
parsed_files: Cow::Borrowed(parsed_files),
arith_constraints: Vec::new().into(),
}
}

Expand Down
Loading