From a718e7253fa3a6e9d077a34252cb77a163a98343 Mon Sep 17 00:00:00 2001 From: Emily Herbert <17410721+emilyaherbert@users.noreply.github.com> Date: Wed, 25 Jan 2023 19:26:23 -0600 Subject: [PATCH] Fix bug with replacing the self type. (#3881) This PR fixes a bug with the self type bug updating the `replace_self_type` method on `TypeId` to properly handle nested types. Closes #3863 Co-authored-by: emilyaherbert --- .../ast_node/declaration/function.rs | 4 +- .../ast_node/declaration/impl_trait.rs | 20 +- sway-core/src/type_system/id.rs | 219 +++++++++++++----- .../generic_type_inference/src/main.sw | 5 + .../generic_type_inference/src/utils.sw | 10 + 5 files changed, 189 insertions(+), 69 deletions(-) diff --git a/sway-core/src/semantic_analysis/ast_node/declaration/function.rs b/sway-core/src/semantic_analysis/ast_node/declaration/function.rs index b516a595110..b9c0c88315e 100644 --- a/sway-core/src/semantic_analysis/ast_node/declaration/function.rs +++ b/sway-core/src/semantic_analysis/ast_node/declaration/function.rs @@ -112,13 +112,13 @@ impl ty::TyFunctionDeclaration { // If there are no implicit block returns, then we do not want to type check them, so we // stifle the errors. If there _are_ implicit block returns, we want to type_check them. let (body, _implicit_block_return) = { - let fn_ctx = ctx + let ctx = ctx .by_ref() .with_purity(purity) .with_help_text("Function body's return type does not match up with its return type annotation.") .with_type_annotation(return_type); check!( - ty::TyCodeBlock::type_check(fn_ctx, body), + ty::TyCodeBlock::type_check(ctx, body), ( ty::TyCodeBlock { contents: vec![] }, type_engine.insert(decl_engine, TypeInfo::ErrorRecovery) diff --git a/sway-core/src/semantic_analysis/ast_node/declaration/impl_trait.rs b/sway-core/src/semantic_analysis/ast_node/declaration/impl_trait.rs index 9068188affd..01bd5d6a358 100644 --- a/sway-core/src/semantic_analysis/ast_node/declaration/impl_trait.rs +++ b/sway-core/src/semantic_analysis/ast_node/declaration/impl_trait.rs @@ -841,9 +841,6 @@ fn type_check_impl_method( }); } - impl_method_param - .type_id - .replace_self_type(engines, self_type); if !type_engine.get(impl_method_param.type_id).eq( &type_engine.get(impl_method_signature_param.type_id), engines, @@ -909,9 +906,6 @@ fn type_check_impl_method( (true, true) | (false, false) => (), // no payability mismatch } - impl_method - .return_type - .replace_self_type(engines, self_type); if !type_engine .get(impl_method.return_type) .eq(&type_engine.get(impl_method_signature.return_type), engines) @@ -948,13 +942,13 @@ fn type_check_impl_method( .cloned() .map(|x| WithEngines::new(x, engines)) .collect(); - let unconstrained_type_parameters_in_the_type: HashSet> = ctx - .self_type() - .unconstrained_type_parameters(engines, impl_type_parameters) - .into_iter() - .cloned() - .map(|x| WithEngines::new(x, engines)) - .collect::>(); + let unconstrained_type_parameters_in_the_type: HashSet> = + self_type + .unconstrained_type_parameters(engines, impl_type_parameters) + .into_iter() + .cloned() + .map(|x| WithEngines::new(x, engines)) + .collect::>(); let mut unconstrained_type_parameters_to_be_added = unconstrained_type_parameters_in_this_function .difference(&unconstrained_type_parameters_in_the_type) diff --git a/sway-core/src/type_system/id.rs b/sway-core/src/type_system/id.rs index 0d3188ddfa8..35442446bda 100644 --- a/sway-core/src/type_system/id.rs +++ b/sway-core/src/type_system/id.rs @@ -77,67 +77,178 @@ impl CollectTypesMetadata for TypeId { impl ReplaceSelfType for TypeId { fn replace_self_type(&mut self, engines: Engines<'_>, self_type: TypeId) { - match engines.te().get(*self) { - TypeInfo::SelfType => { - *self = self_type; - } - TypeInfo::Enum { - mut type_parameters, - mut variant_types, - .. - } => { - for type_parameter in type_parameters.iter_mut() { - type_parameter.replace_self_type(engines, self_type); - } - for variant_type in variant_types.iter_mut() { - variant_type.replace_self_type(engines, self_type); - } - } - TypeInfo::Struct { - mut type_parameters, - mut fields, - .. - } => { - for type_parameter in type_parameters.iter_mut() { - type_parameter.replace_self_type(engines, self_type); + fn helper(type_id: TypeId, engines: Engines<'_>, self_type: TypeId) -> Option { + let type_engine = engines.te(); + let decl_engine = engines.de(); + match type_engine.get(type_id) { + TypeInfo::SelfType => Some(self_type), + TypeInfo::Enum { + type_parameters, + variant_types, + name, + } => { + let mut need_to_create_new = false; + let variant_types = variant_types + .into_iter() + .map(|mut variant| { + if let Some(type_id) = helper(variant.type_id, engines, self_type) { + need_to_create_new = true; + variant.type_id = type_id; + } + variant + }) + .collect::>(); + let type_parameters = type_parameters + .into_iter() + .map(|mut type_param| { + if let Some(type_id) = helper(type_param.type_id, engines, self_type) { + need_to_create_new = true; + type_param.type_id = type_id; + } + type_param + }) + .collect::>(); + if need_to_create_new { + Some(type_engine.insert( + decl_engine, + TypeInfo::Enum { + variant_types, + type_parameters, + name, + }, + )) + } else { + None + } } - for field in fields.iter_mut() { - field.replace_self_type(engines, self_type); + TypeInfo::Struct { + type_parameters, + fields, + name, + } => { + let mut need_to_create_new = false; + let fields = fields + .into_iter() + .map(|mut field| { + if let Some(type_id) = helper(field.type_id, engines, self_type) { + need_to_create_new = true; + field.type_id = type_id; + } + field + }) + .collect::>(); + let type_parameters = type_parameters + .into_iter() + .map(|mut type_param| { + if let Some(type_id) = helper(type_param.type_id, engines, self_type) { + need_to_create_new = true; + type_param.type_id = type_id; + } + type_param + }) + .collect::>(); + if need_to_create_new { + Some(type_engine.insert( + decl_engine, + TypeInfo::Struct { + fields, + name, + type_parameters, + }, + )) + } else { + None + } } - } - TypeInfo::Tuple(mut type_arguments) => { - for type_argument in type_arguments.iter_mut() { - type_argument.replace_self_type(engines, self_type); + TypeInfo::Tuple(fields) => { + let mut need_to_create_new = false; + let fields = fields + .into_iter() + .map(|mut field| { + if let Some(type_id) = helper(field.type_id, engines, self_type) { + need_to_create_new = true; + field.type_id = type_id; + } + field + }) + .collect::>(); + if need_to_create_new { + Some(type_engine.insert(decl_engine, TypeInfo::Tuple(fields))) + } else { + None + } } - } - TypeInfo::Custom { type_arguments, .. } => { - if let Some(mut type_arguments) = type_arguments { - for type_argument in type_arguments.iter_mut() { - type_argument.replace_self_type(engines, self_type); + TypeInfo::Custom { + name, + type_arguments, + } => { + let mut need_to_create_new = false; + let type_arguments = type_arguments.map(|type_arguments| { + type_arguments + .into_iter() + .map(|mut type_arg| { + if let Some(type_id) = helper(type_arg.type_id, engines, self_type) + { + need_to_create_new = true; + type_arg.type_id = type_id; + } + type_arg + }) + .collect::>() + }); + if need_to_create_new { + Some(type_engine.insert( + decl_engine, + TypeInfo::Custom { + name, + type_arguments, + }, + )) + } else { + None } } - } - TypeInfo::Array(mut type_id, _) => { - type_id.replace_self_type(engines, self_type); - } - TypeInfo::Storage { mut fields } => { - for field in fields.iter_mut() { - field.replace_self_type(engines, self_type); + TypeInfo::Array(mut elem_ty, count) => helper(elem_ty.type_id, engines, self_type) + .map(|type_id| { + elem_ty.type_id = type_id; + type_engine.insert(decl_engine, TypeInfo::Array(elem_ty, count)) + }), + TypeInfo::Storage { fields } => { + let mut need_to_create_new = false; + let fields = fields + .into_iter() + .map(|mut field| { + if let Some(type_id) = helper(field.type_id, engines, self_type) { + need_to_create_new = true; + field.type_id = type_id; + } + field + }) + .collect::>(); + if need_to_create_new { + Some(type_engine.insert(decl_engine, TypeInfo::Storage { fields })) + } else { + None + } } + TypeInfo::Unknown + | TypeInfo::UnknownGeneric { .. } + | TypeInfo::Str(_) + | TypeInfo::UnsignedInteger(_) + | TypeInfo::Boolean + | TypeInfo::ContractCaller { .. } + | TypeInfo::B256 + | TypeInfo::Numeric + | TypeInfo::RawUntypedPtr + | TypeInfo::RawUntypedSlice + | TypeInfo::Contract + | TypeInfo::ErrorRecovery + | TypeInfo::Placeholder(_) => None, } - TypeInfo::Unknown - | TypeInfo::UnknownGeneric { .. } - | TypeInfo::Str(_) - | TypeInfo::UnsignedInteger(_) - | TypeInfo::Boolean - | TypeInfo::ContractCaller { .. } - | TypeInfo::B256 - | TypeInfo::Numeric - | TypeInfo::RawUntypedPtr - | TypeInfo::RawUntypedSlice - | TypeInfo::Contract - | TypeInfo::ErrorRecovery - | TypeInfo::Placeholder(_) => {} + } + + if let Some(type_id) = helper(*self, engines, self_type) { + *self = type_id; } } } diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/generic_type_inference/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/language/generic_type_inference/src/main.sw index ad58849cc37..62dbf3915bd 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/language/generic_type_inference/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/should_pass/language/generic_type_inference/src/main.sw @@ -46,6 +46,11 @@ fn simple_option_generics_test() { assert(get_an_option::().is_none()); } +fn test_try_from() { + let x = u64::try_from(7); + assert(x.unwrap() == 42); +} + fn main() { sell_product(); simple_vec_test(); diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/generic_type_inference/src/utils.sw b/test/src/e2e_vm_tests/test_programs/should_pass/language/generic_type_inference/src/utils.sw index d91973b7e12..622bead485c 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/language/generic_type_inference/src/utils.sw +++ b/test/src/e2e_vm_tests/test_programs/should_pass/language/generic_type_inference/src/utils.sw @@ -11,3 +11,13 @@ pub fn vec_from(vals: [u32; 3]) -> Vec { pub fn get_an_option() -> Option { Option::None } + +pub trait TryFrom { + fn try_from(b: T) -> Option; +} + +impl TryFrom for u64 { + fn try_from(b: u64) -> Option { + Option::Some(42) + } +}