From 9f46e5e7b4401d9fe1569d084d7ad1fcdfe48c7c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 6 Aug 2024 12:56:56 -0300 Subject: [PATCH] Use correct locations for errors --- .../src/hir/comptime/interpreter/builtin.rs | 192 ++++++++++-------- .../interpreter/builtin/builtin_helpers.rs | 18 +- .../src/hir/comptime/interpreter/foreign.rs | 7 +- 3 files changed, 125 insertions(+), 92 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index 0bed66bf699..db6df1df1b7 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -109,7 +109,7 @@ fn array_len( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let argument = check_one_argument(arguments, location)?; + let (argument, argument_location) = check_one_argument(arguments, location)?; match argument { Value::Array(values, _) | Value::Slice(values, _) => Ok(Value::U32(values.len() as u32)), @@ -117,7 +117,7 @@ fn array_len( let type_var = Box::new(interner.next_type_variable()); let expected = Type::Array(type_var.clone(), type_var); let actual = value.get_type().into_owned(); - Err(InterpreterError::TypeMismatch { expected, actual, location }) + Err(InterpreterError::TypeMismatch { expected, actual, location: argument_location }) } } } @@ -127,7 +127,7 @@ fn as_slice( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let array = check_one_argument(arguments, location)?; + let (array, array_location) = check_one_argument(arguments, location)?; match array { Value::Array(values, Type::Array(_, typ)) => Ok(Value::Slice(values, Type::Slice(typ))), @@ -135,7 +135,7 @@ fn as_slice( let type_var = Box::new(interner.next_type_variable()); let expected = Type::Array(type_var.clone(), type_var); let actual = value.get_type().into_owned(); - Err(InterpreterError::TypeMismatch { expected, actual, location }) + Err(InterpreterError::TypeMismatch { expected, actual, location: array_location }) } } } @@ -145,9 +145,9 @@ fn slice_push_back( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let (slice, element) = check_two_arguments(arguments, location)?; + let ((slice, slice_location), (element, _)) = check_two_arguments(arguments, location)?; - let (mut values, typ) = get_slice(interner, slice, location)?; + let (mut values, typ) = get_slice(interner, slice, slice_location)?; values.push_back(element); Ok(Value::Slice(values, typ)) } @@ -158,14 +158,18 @@ fn struct_def_as_type( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let argument = check_one_argument(arguments, location)?; + let (argument, argument_location) = check_one_argument(arguments, location)?; let struct_def = match argument { Value::StructDefinition(id) => id, value => { let expected = Type::Quoted(QuotedType::StructDefinition); let actual = value.get_type().into_owned(); - return Err(InterpreterError::TypeMismatch { expected, location, actual }); + return Err(InterpreterError::TypeMismatch { + expected, + location: argument_location, + actual, + }); } }; @@ -186,14 +190,18 @@ fn struct_def_generics( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let argument = check_one_argument(arguments, location)?; + let (argument, argument_location) = check_one_argument(arguments, location)?; let struct_def = match argument { Value::StructDefinition(id) => id, value => { let expected = Type::Quoted(QuotedType::StructDefinition); let actual = value.get_type().into_owned(); - return Err(InterpreterError::TypeMismatch { expected, location, actual }); + return Err(InterpreterError::TypeMismatch { + expected, + location: argument_location, + actual, + }); } }; @@ -214,14 +222,18 @@ fn struct_def_fields( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let argument = check_one_argument(arguments, location)?; + let (argument, argument_location) = check_one_argument(arguments, location)?; let struct_def = match argument { Value::StructDefinition(id) => id, value => { let expected = Type::Quoted(QuotedType::StructDefinition); let actual = value.get_type().into_owned(); - return Err(InterpreterError::TypeMismatch { expected, location, actual }); + return Err(InterpreterError::TypeMismatch { + expected, + location: argument_location, + actual, + }); } }; @@ -248,10 +260,11 @@ fn slice_remove( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let (slice, index) = check_two_arguments(arguments, location)?; + let ((slice, slice_location), (index, index_location)) = + check_two_arguments(arguments, location)?; - let index = get_u32(index, location)? as usize; - let (mut values, typ) = get_slice(interner, slice, location)?; + let index = get_u32(index, index_location)? as usize; + let (mut values, typ) = get_slice(interner, slice, slice_location)?; if values.is_empty() { return failing_constraint("slice_remove called on empty slice", location); @@ -274,9 +287,9 @@ fn slice_push_front( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let (slice, element) = check_two_arguments(arguments, location)?; + let ((slice, slice_location), (element, _)) = check_two_arguments(arguments, location)?; - let (mut values, typ) = get_slice(interner, slice, location)?; + let (mut values, typ) = get_slice(interner, slice, slice_location)?; values.push_front(element); Ok(Value::Slice(values, typ)) } @@ -286,9 +299,9 @@ fn slice_pop_front( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let argument = check_one_argument(arguments, location)?; + let (argument, argument_location) = check_one_argument(arguments, location)?; - let (mut values, typ) = get_slice(interner, argument, location)?; + let (mut values, typ) = get_slice(interner, argument, argument_location)?; match values.pop_front() { Some(element) => Ok(Value::Tuple(vec![element, Value::Slice(values, typ)])), None => failing_constraint("slice_pop_front called on empty slice", location), @@ -300,9 +313,9 @@ fn slice_pop_back( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let argument = check_one_argument(arguments, location)?; + let (argument, argument_location) = check_one_argument(arguments, location)?; - let (mut values, typ) = get_slice(interner, argument, location)?; + let (mut values, typ) = get_slice(interner, argument, argument_location)?; match values.pop_back() { Some(element) => Ok(Value::Tuple(vec![Value::Slice(values, typ), element])), None => failing_constraint("slice_pop_back called on empty slice", location), @@ -314,10 +327,11 @@ fn slice_insert( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let (slice, index, element) = check_three_arguments(arguments, location)?; + let ((slice, slice_location), (index, index_location), (element, _)) = + check_three_arguments(arguments, location)?; - let index = get_u32(index, location)? as usize; - let (mut values, typ) = get_slice(interner, slice, location)?; + let index = get_u32(index, index_location)? as usize; + let (mut values, typ) = get_slice(interner, slice, slice_location)?; values.insert(index, element); Ok(Value::Slice(values, typ)) } @@ -329,10 +343,10 @@ fn quoted_as_module( return_type: Type, location: Location, ) -> IResult { - let argument = check_one_argument(arguments, location)?; + let (argument, argument_location) = check_one_argument(arguments, location)?; - let tokens = get_quoted(argument, location)?; - let quoted = add_token_spans(tokens.clone(), location.span); + let tokens = get_quoted(argument, argument_location)?; + let quoted = add_token_spans(tokens.clone(), argument_location.span); let option_value = parser::path_no_turbofish() .parse(quoted) @@ -353,10 +367,10 @@ fn quoted_as_trait_constraint( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let argument = check_one_argument(arguments, location)?; + let (argument, argument_location) = check_one_argument(arguments, location)?; - let tokens = get_quoted(argument, location)?; - let quoted = add_token_spans(tokens.clone(), location.span); + let tokens = get_quoted(argument, argument_location)?; + let quoted = add_token_spans(tokens.clone(), argument_location.span); let trait_bound = parser::trait_bound().parse(quoted).map_err(|mut errors| { let error = errors.swap_remove(0); @@ -379,10 +393,10 @@ fn quoted_as_type( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let argument = check_one_argument(arguments, location)?; + let (argument, argument_location) = check_one_argument(arguments, location)?; - let tokens = get_quoted(argument, location)?; - let quoted = add_token_spans(tokens.clone(), location.span); + let tokens = get_quoted(argument, argument_location)?; + let quoted = add_token_spans(tokens.clone(), argument_location.span); let typ = parser::parse_type().parse(quoted).map_err(|mut errors| { let error = errors.swap_remove(0); @@ -508,8 +522,8 @@ fn type_as( where F: FnOnce(Type) -> Option, { - let value = check_one_argument(arguments, location)?; - let typ = get_type(value, location)?; + let (value, value_location) = check_one_argument(arguments, location)?; + let typ = get_type(value, value_location)?; let option_value = f(typ); @@ -518,33 +532,34 @@ where // fn type_eq(_first: Type, _second: Type) -> bool fn type_eq(arguments: Vec<(Value, Location)>, location: Location) -> IResult { - let (self_type, other_type) = check_two_arguments(arguments, location)?; + let ((self_type, self_type_location), (other_type, other_type_location)) = + check_two_arguments(arguments, location)?; - let self_type = get_type(self_type, location)?; - let other_type = get_type(other_type, location)?; + let self_type = get_type(self_type, self_type_location)?; + let other_type = get_type(other_type, other_type_location)?; Ok(Value::Bool(self_type == other_type)) } // fn is_bool(self) -> bool fn type_is_bool(arguments: Vec<(Value, Location)>, location: Location) -> IResult { - let value = check_one_argument(arguments, location)?; - let typ = get_type(value, location)?; + let (value, value_location) = check_one_argument(arguments, location)?; + let typ = get_type(value, value_location)?; Ok(Value::Bool(matches!(typ, Type::Bool))) } // fn is_field(self) -> bool fn type_is_field(arguments: Vec<(Value, Location)>, location: Location) -> IResult { - let value = check_one_argument(arguments, location)?; - let typ = get_type(value, location)?; + let (value, value_location) = check_one_argument(arguments, location)?; + let typ = get_type(value, value_location)?; Ok(Value::Bool(matches!(typ, Type::FieldElement))) } // fn type_of(x: T) -> Type fn type_of(arguments: Vec<(Value, Location)>, location: Location) -> IResult { - let value = check_one_argument(arguments, location)?; + let (value, _) = check_one_argument(arguments, location)?; let typ = value.get_type().into_owned(); Ok(Value::Type(typ)) } @@ -555,9 +570,9 @@ fn trait_constraint_hash( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let argument = check_one_argument(arguments, location)?; + let (argument, argument_location) = check_one_argument(arguments, location)?; - let bound = get_trait_constraint(argument, location)?; + let bound = get_trait_constraint(argument, argument_location)?; let mut hasher = std::collections::hash_map::DefaultHasher::new(); bound.hash(&mut hasher); @@ -572,10 +587,11 @@ fn trait_constraint_eq( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let (value_a, value_b) = check_two_arguments(arguments, location)?; + let ((value_a, value_a_location), (value_b, value_b_location)) = + check_two_arguments(arguments, location)?; - let constraint_a = get_trait_constraint(value_a, location)?; - let constraint_b = get_trait_constraint(value_b, location)?; + let constraint_a = get_trait_constraint(value_a, value_a_location)?; + let constraint_b = get_trait_constraint(value_b, value_b_location)?; Ok(Value::Bool(constraint_a == constraint_b)) } @@ -698,8 +714,8 @@ fn function_def_name( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let self_argument = check_one_argument(arguments, location)?; - let func_id = get_function_def(self_argument, location)?; + let (self_argument, self_argument_location) = check_one_argument(arguments, location)?; + let func_id = get_function_def(self_argument, self_argument_location)?; let func_meta = interner.function_meta(&func_id); let name = &interner.definition(func_meta.name.id).name; let tokens = Rc::new(vec![Token::Ident(name.clone())]); @@ -712,8 +728,8 @@ fn function_def_parameters( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let self_argument = check_one_argument(arguments, location)?; - let func_id = get_function_def(self_argument, location)?; + let (self_argument, self_argument_location) = check_one_argument(arguments, location)?; + let func_id = get_function_def(self_argument, self_argument_location)?; let func_meta = interner.function_meta(&func_id); let parameters = func_meta @@ -740,8 +756,8 @@ fn function_def_return_type( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let self_argument = check_one_argument(arguments, location)?; - let func_id = get_function_def(self_argument, location)?; + let (self_argument, self_argument_location) = check_one_argument(arguments, location)?; + let func_id = get_function_def(self_argument, self_argument_location)?; let func_meta = interner.function_meta(&func_id); Ok(Value::Type(func_meta.return_type().follow_bindings())) @@ -753,8 +769,9 @@ fn function_def_set_body( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let (self_argument, body_argument) = check_two_arguments(arguments, location)?; - let func_id = get_function_def(self_argument, location)?; + let ((self_argument, self_argument_location), (body_argument, body_argument_location)) = + check_two_arguments(arguments, location)?; + let func_id = get_function_def(self_argument, self_argument_location)?; let func_meta = interpreter.elaborator.interner.function_meta(&func_id); match func_meta.function_body { @@ -764,8 +781,8 @@ fn function_def_set_body( } } - let body_tokens = get_quoted(body_argument, location)?; - let mut body_quoted = add_token_spans(body_tokens.clone(), location.span); + let body_tokens = get_quoted(body_argument, body_argument_location)?; + let mut body_quoted = add_token_spans(body_tokens.clone(), body_argument_location.span); // Surround the body in `{ ... }` so we can parse it as a block body_quoted.0.insert(0, SpannedToken::new(Token::LeftBrace, location.span)); @@ -796,9 +813,12 @@ fn function_def_set_parameters( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let (self_argument, parameters_argument) = check_two_arguments(arguments, location)?; + let ( + (self_argument, self_argument_location), + (parameters_argument, parameters_argument_location), + ) = check_two_arguments(arguments, location)?; - let func_id = get_function_def(self_argument, location)?; + let func_id = get_function_def(self_argument, self_argument_location)?; let func_meta = interpreter.elaborator.interner.function_meta(&func_id); match func_meta.function_body { FunctionBody::Unresolved(_, _, _) => (), @@ -807,8 +827,11 @@ fn function_def_set_parameters( } } - let (input_parameters, _type) = - get_slice(interpreter.elaborator.interner, parameters_argument, location)?; + let (input_parameters, _type) = get_slice( + interpreter.elaborator.interner, + parameters_argument, + parameters_argument_location, + )?; // What follows is very similar to what happens in Elaborator::define_function_meta let mut parameters = Vec::new(); @@ -816,10 +839,15 @@ fn function_def_set_parameters( let mut parameter_idents = Vec::new(); for input_parameter in input_parameters { - let mut tuple = get_tuple(interpreter.elaborator.interner, input_parameter, location)?; - let parameter_type = get_type(tuple.pop().unwrap(), location)?; - let parameter_name_tokens = get_quoted(tuple.pop().unwrap(), location)?; - let parameter_name_quoted = add_token_spans(parameter_name_tokens.clone(), location.span); + let mut tuple = get_tuple( + interpreter.elaborator.interner, + input_parameter, + parameters_argument_location, + )?; + let parameter_type = get_type(tuple.pop().unwrap(), parameters_argument_location)?; + let parameter_name_tokens = get_quoted(tuple.pop().unwrap(), parameters_argument_location)?; + let parameter_name_quoted = + add_token_spans(parameter_name_tokens.clone(), parameters_argument_location.span); let parameter_pattern = parser::pattern().parse(parameter_name_quoted).map_err(|mut errors| { let error = errors.swap_remove(0); @@ -877,10 +905,13 @@ fn function_def_set_return_type( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let (self_argument, return_type_argument) = check_two_arguments(arguments, location)?; - let return_type = get_type(return_type_argument, location)?; + let ( + (self_argument, self_argument_location), + (return_type_argument, return_type_argument_location), + ) = check_two_arguments(arguments, location)?; + let return_type = get_type(return_type_argument, return_type_argument_location)?; - let func_id = get_function_def(self_argument, location)?; + let func_id = get_function_def(self_argument, self_argument_location)?; let func_meta = interpreter.elaborator.interner.function_meta(&func_id); match func_meta.function_body { FunctionBody::Unresolved(_, _, _) => (), @@ -923,8 +954,8 @@ fn module_functions( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let self_argument = check_one_argument(arguments, location)?; - let module_id = get_module(self_argument, location)?; + let (self_argument, self_argument_location) = check_one_argument(arguments, location)?; + let module_id = get_module(self_argument, self_argument_location)?; let module_data = interpreter.elaborator.get_module(module_id); let func_ids = module_data .value_definitions() @@ -948,8 +979,8 @@ fn module_is_contract( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let self_argument = check_one_argument(arguments, location)?; - let module_id = get_module(self_argument, location)?; + let (self_argument, self_argument_location) = check_one_argument(arguments, location)?; + let module_id = get_module(self_argument, self_argument_location)?; Ok(Value::Bool(interpreter.elaborator.module_is_contract(module_id))) } @@ -959,8 +990,8 @@ fn module_name( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let self_argument = check_one_argument(arguments, location)?; - let module_id = get_module(self_argument, location)?; + let (self_argument, self_argument_location) = check_one_argument(arguments, location)?; + let module_id = get_module(self_argument, self_argument_location)?; let name = &interner.module_attributes(&module_id).name; let tokens = Rc::new(vec![Token::Ident(name.clone())]); Ok(Value::Quoted(tokens)) @@ -1032,10 +1063,11 @@ fn modulus_num_bits( // fn quoted_eq(_first: Quoted, _second: Quoted) -> bool fn quoted_eq(arguments: Vec<(Value, Location)>, location: Location) -> IResult { - let (self_value, other_value) = check_two_arguments(arguments, location)?; + let ((self_value, self_value_location), (other_value, other_value_location)) = + check_two_arguments(arguments, location)?; - let self_quoted = get_quoted(self_value, location)?; - let other_quoted = get_quoted(other_value, location)?; + let self_quoted = get_quoted(self_value, self_value_location)?; + let other_quoted = get_quoted(other_value, other_value_location)?; Ok(Value::Bool(self_quoted == other_quoted)) } @@ -1045,9 +1077,9 @@ fn trait_def_as_trait_constraint( arguments: Vec<(Value, Location)>, location: Location, ) -> Result { - let argument = check_one_argument(arguments, location)?; + let (argument, argument_location) = check_one_argument(arguments, location)?; - let trait_id = get_trait_def(argument, location)?; + let trait_id = get_trait_def(argument, argument_location)?; let the_trait = interner.get_trait(trait_id); let trait_generics = vecmap(&the_trait.generics, |generic| { Type::NamedGeneric(generic.type_var.clone(), generic.name.clone(), generic.kind.clone()) diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs index f8f75f7f90d..99379fc4e33 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs @@ -32,20 +32,20 @@ pub(crate) fn check_argument_count( pub(crate) fn check_one_argument( mut arguments: Vec<(Value, Location)>, location: Location, -) -> IResult { +) -> IResult<(Value, Location)> { check_argument_count(1, &arguments, location)?; - Ok(arguments.pop().unwrap().0) + Ok(arguments.pop().unwrap()) } pub(crate) fn check_two_arguments( mut arguments: Vec<(Value, Location)>, location: Location, -) -> IResult<(Value, Value)> { +) -> IResult<((Value, Location), (Value, Location))> { check_argument_count(2, &arguments, location)?; - let argument2 = arguments.pop().unwrap().0; - let argument1 = arguments.pop().unwrap().0; + let argument2 = arguments.pop().unwrap(); + let argument1 = arguments.pop().unwrap(); Ok((argument1, argument2)) } @@ -53,12 +53,12 @@ pub(crate) fn check_two_arguments( pub(crate) fn check_three_arguments( mut arguments: Vec<(Value, Location)>, location: Location, -) -> IResult<(Value, Value, Value)> { +) -> IResult<((Value, Location), (Value, Location), (Value, Location))> { check_argument_count(3, &arguments, location)?; - let argument3 = arguments.pop().unwrap().0; - let argument2 = arguments.pop().unwrap().0; - let argument1 = arguments.pop().unwrap().0; + let argument3 = arguments.pop().unwrap(); + let argument2 = arguments.pop().unwrap(); + let argument1 = arguments.pop().unwrap(); Ok((argument1, argument2, argument3)) } diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs index d6e4553d142..3db58ee5b3f 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs @@ -31,10 +31,11 @@ fn poseidon2_permutation( arguments: Vec<(Value, Location)>, location: Location, ) -> IResult { - let (input, state_length) = check_two_arguments(arguments, location)?; + let ((input, input_location), (state_length, state_length_location)) = + check_two_arguments(arguments, location)?; - let (input, typ) = get_array(interner, input, location)?; - let state_length = get_u32(state_length, location)?; + let (input, typ) = get_array(interner, input, input_location)?; + let state_length = get_u32(state_length, state_length_location)?; let input = try_vecmap(input, |integer| get_field(integer, location))?;