From 704669ecde2463484e7a43c00c4cb61320571a4d Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Sat, 9 Jan 2021 00:11:32 +0100 Subject: [PATCH] fix: Make VmType generate correct variant constructors The opaque return type was likely a quick hack that went unnoticed. I also had to force caching on the enum since the `Symbol` values created for the variants and the returned enum will otherwise be different between multiple `VmType::make_type` invocations (unless Fixes #901 --- base/src/types/mod.rs | 9 ++++---- codegen/src/arena_clone.rs | 19 ++++++++++------- codegen/src/vm_type.rs | 42 ++++++++++++++++++++++++++++++-------- tests/vm.rs | 42 +++++++++++++++++++++++++++++++++++++- vm/src/api/typ.rs | 23 +++++++++++++++++++-- 5 files changed, 112 insertions(+), 23 deletions(-) diff --git a/base/src/types/mod.rs b/base/src/types/mod.rs index 568e454a8b..ead9d30c80 100644 --- a/base/src/types/mod.rs +++ b/base/src/types/mod.rs @@ -860,6 +860,7 @@ impl Field { pub fn ctor_with( context: &mut (impl TypeContext + ?Sized), + enum_type: KindedIdent, ctor_name: SpId, elems: J, ) -> Self @@ -868,22 +869,22 @@ impl Field { J::IntoIter: DoubleEndedIterator, T: TypePtr, { - let opaque = context.opaque(); - let typ = context.function_type(ArgType::Constructor, elems, opaque); + let enum_type = context.ident(enum_type); + let typ = context.function_type(ArgType::Constructor, elems, enum_type); Field { name: ctor_name, typ, } } - pub fn ctor(ctor_name: SpId, elems: J) -> Self + pub fn ctor(enum_type: KindedIdent, ctor_name: SpId, elems: J) -> Self where J: IntoIterator, J::IntoIter: DoubleEndedIterator, T: TypeExt + From>, T::Types: Default + Extend, { - let typ = Type::function_type(ArgType::Constructor, elems, Type::opaque()); + let typ = Type::function_type(ArgType::Constructor, elems, Type::ident(enum_type)); Field { name: ctor_name, typ, diff --git a/codegen/src/arena_clone.rs b/codegen/src/arena_clone.rs index 26125ee16b..4319e7f97d 100644 --- a/codegen/src/arena_clone.rs +++ b/codegen/src/arena_clone.rs @@ -72,21 +72,21 @@ fn gen_impl(container: &Container, ident: Ident, generics: Generics, data: &Data }, Data::Enum(ref enum_) => { let variants = enum_.variants.iter().map(|variant| { - let ident = variant.ident.to_string(); + let variant_ident = variant.ident.to_string(); match variant.fields { Fields::Named(ref fields) => { let fields = fields.named.iter().map(|field| { - let ident = field.ident.as_ref().unwrap().to_string(); + let variant_ident = field.variant_ident.as_ref().unwrap().to_string(); let typ = &field.ty; quote! { _gluon_base::types::Field { - name: _gluon_base::symbol::Symbol::from(#ident), + name: _gluon_base::symbol::Symbol::from(#variant_ident), typ: <#typ as _gluon_api::VmType>::make_type(vm), } } }); quote! {{ - let ctor_name = _gluon_base::symbol::Symbol::from(#ident); + let ctor_name = _gluon_base::symbol::Symbol::from(#variant_ident); let typ = _gluon_base::types::Type::record( vec![], vec![#(#fields),*], @@ -105,7 +105,7 @@ fn gen_impl(container: &Container, ident: Ident, generics: Generics, data: &Data } }); quote! {{ - let ctor_name = _gluon_base::symbol::Symbol::from(#ident); + let ctor_name = _gluon_base::symbol::Symbol::from(#variant_ident); _gluon_base::types::Field::ctor( ctor_name, vec![#(#args),*], @@ -113,9 +113,14 @@ fn gen_impl(container: &Container, ident: Ident, generics: Generics, data: &Data }} } Fields::Unit => quote! {{ - let ctor_name = _gluon_base::symbol::Symbol::from(#ident); + let ctor_name = _gluon_base::symbol::Symbol::from(#variant_ident); _gluon_base::types::Field::ctor( - ctor_name, vec![], + _gluon_base::ast::TypedIdent { + name: _gluon_base::symbol::Symbol::from(#ident), + typ: vm.global_env().type_cache().kind_cache.typ(), + }, + ctor_name, + vec![], ) }}, } diff --git a/codegen/src/vm_type.rs b/codegen/src/vm_type.rs index c54e70a067..e8135332cb 100644 --- a/codegen/src/vm_type.rs +++ b/codegen/src/vm_type.rs @@ -51,6 +51,10 @@ fn gen_impl(container: &Container, ident: Ident, generics: Generics, data: &Data }, }; + // Enums create symbols so we need to cache the created typ or multiple invocations will return + // different types + let mut is_enum = false; + let make_type_impl = match container.vm_type { Some(ref gluon_type) => { let type_application = gen_type_application(&generics); @@ -101,27 +105,36 @@ fn gen_impl(container: &Container, ident: Ident, generics: Generics, data: &Data Fields::Unit => quote!(_gluon_base::types::Type::unit()), }, Data::Enum(ref enum_) => { + is_enum = true; let variants = enum_.variants.iter().map(|variant| { - let ident = variant.ident.to_string(); + let variant_ident = variant.ident.to_string(); match variant.fields { Fields::Named(ref fields) => { let fields = fields.named.iter().map(|field| { - let ident = field.ident.as_ref().unwrap().to_string(); + let variant_ident = field.ident.as_ref().unwrap().to_string(); let typ = &field.ty; quote! { _gluon_base::types::Field { - name: _gluon_base::symbol::Symbol::from(#ident), + name: _gluon_base::symbol::Symbol::from(#variant_ident), typ: <#typ as _gluon_api::VmType>::make_type(vm), } } }); quote! {{ - let ctor_name = _gluon_base::symbol::Symbol::from(#ident); + let ctor_name = _gluon_base::symbol::Symbol::from(#variant_ident); let typ = _gluon_base::types::Type::record( + _gluon_base::ast::TypedIdent { + name:_gluon_base::symbol::Symbol::from(#ident); + typ: vm.global_env().type_cache().kind_cache.typ() + }, vec![], vec![#(#fields),*], ); _gluon_base::types::Field::ctor( + _gluon_base::ast::TypedIdent { + name: type_name.clone(), + typ: vm.global_env().type_cache().kind_cache.typ() + }, ctor_name, vec![typ], ) @@ -135,17 +148,26 @@ fn gen_impl(container: &Container, ident: Ident, generics: Generics, data: &Data } }); quote! {{ - let ctor_name = _gluon_base::symbol::Symbol::from(#ident); + let ctor_name = _gluon_base::symbol::Symbol::from(#variant_ident); _gluon_base::types::Field::ctor( + _gluon_base::ast::TypedIdent { + name: type_name.clone(), + typ: vm.global_env().type_cache().kind_cache.typ() + }, ctor_name, vec![#(#args),*], ) }} } Fields::Unit => quote! {{ - let ctor_name = _gluon_base::symbol::Symbol::from(#ident); + let ctor_name = _gluon_base::symbol::Symbol::from(#variant_ident); _gluon_base::types::Field::ctor( - ctor_name, vec![], + _gluon_base::ast::TypedIdent { + name: type_name.clone(), + typ: vm.global_env().type_cache().kind_cache.typ() + }, + ctor_name, + vec![], ) }}, } @@ -153,6 +175,7 @@ fn gen_impl(container: &Container, ident: Ident, generics: Generics, data: &Data //---------------------------------------------------- }); + quote! { _gluon_base::types::Type::variant( vec![#(#variants),*] @@ -173,7 +196,7 @@ fn gen_impl(container: &Container, ident: Ident, generics: Generics, data: &Data let dummy_const = Ident::new(&format!("_IMPL_VM_TYPE_FOR_{}", ident), Span::call_site()); - let make_type_impl = if container.newtype { + let make_type_impl = if container.newtype || is_enum { let type_application = gen_type_application(&generics); let generic_params = map_type_params(&generics, |param| { let lower_param = param.to_string().to_ascii_lowercase(); @@ -186,8 +209,9 @@ fn gen_impl(container: &Container, ident: Ident, generics: Generics, data: &Data let ty = if let Some(ty) = vm.get_cache_alias(stringify!(#ident)) { ty } else { + let type_name = _gluon_base::symbol::Symbol::from(stringify!(#ident)); let ty = _gluon_base::types::Alias::new( - _gluon_base::symbol::Symbol::from(stringify!(#ident)), + type_name.clone(), vec![#(#generic_params),*], #make_type_impl, ); diff --git a/tests/vm.rs b/tests/vm.rs index 8e6fb1734a..33ac900a43 100644 --- a/tests/vm.rs +++ b/tests/vm.rs @@ -10,14 +10,17 @@ use crate::support::*; use gluon::{ base::{pos::BytePos, source::Source, types::Type}, - vm, + import::add_extern_module, + record, vm, vm::{ api::{FunctionRef, Hole, OpaqueValue, ValueRef, IO}, channel::Sender, thread::Thread, + ExternModule, }, Error, ThreadExt, }; +use gluon_codegen::{Getable, Pushable, VmType}; test_expr! { pass_function_value, r" @@ -1103,3 +1106,40 @@ let f a = { f } " } + +#[derive(Clone, Debug, VmType, Pushable, Getable)] +enum E1 { + S1(S), +} + +#[derive(Clone, Debug, VmType, Pushable, Getable)] +struct S { + e: E2, +} + +#[derive(Clone, Debug, VmType, Pushable, Getable)] +enum E2 { + Num(i32), +} + +#[test] +fn issue_901() { + let _ = ::env_logger::try_init(); + let text = r" +let { E1, S, E2 } = import! test +let e1 = S1 { e = Num 3 } +() +"; + let vm = make_vm(); + add_extern_module(&vm, "test", |vm| { + ExternModule::new( + vm, + record! { + type E1 => E1, + type S => S, + type E2 => E2, + }, + ) + }); + run_expr::>(&vm, text); +} diff --git a/vm/src/api/typ.rs b/vm/src/api/typ.rs index daa06185d2..845e7eac25 100644 --- a/vm/src/api/typ.rs +++ b/vm/src/api/typ.rs @@ -2,8 +2,11 @@ //! //! _This module requires Gluon to be built with the `serde` feature._ -use crate::base::symbol::{Symbol, Symbols}; -use crate::base::types::{ArcType, Field, Type, TypeCache, TypeExt}; +use crate::base::{ + ast::TypedIdent, + symbol::{Symbol, Symbols}, + types::{ArcType, Field, Type, TypeCache, TypeExt}, +}; use crate::api::VmType; use crate::thread::Thread; @@ -521,6 +524,10 @@ impl<'de, 'a> VariantAccess<'de> for Enum<'a, 'de> { fn unit_variant(self) -> Result<()> { self.de.variant = Some(Field::ctor( + TypedIdent { + name: self.de.state.symbols.simple_symbol(self.de.name), + typ: self.de.state.cache.kind_cache.typ(), + }, self.de.state.symbols.simple_symbol(self.variant), vec![], )); @@ -533,6 +540,10 @@ impl<'de, 'a> VariantAccess<'de> for Enum<'a, 'de> { { let value = seed.deserialize(&mut *self.de)?; self.de.variant = Some(Field::ctor( + TypedIdent { + name: self.de.state.symbols.simple_symbol(self.de.name), + typ: self.de.state.cache.kind_cache.typ(), + }, self.de.state.symbols.simple_symbol(self.variant), vec![self.de.typ.take().expect("typ")], )); @@ -551,6 +562,10 @@ impl<'de, 'a> VariantAccess<'de> for Enum<'a, 'de> { ) }; self.de.variant = Some(Field::ctor( + TypedIdent { + name: self.de.state.symbols.simple_symbol(self.de.name), + typ: self.de.state.cache.kind_cache.typ(), + }, self.de.state.symbols.simple_symbol(self.variant), types, )); @@ -569,6 +584,10 @@ impl<'de, 'a> VariantAccess<'de> for Enum<'a, 'de> { ) }; self.de.variant = Some(Field::ctor( + TypedIdent { + name: self.de.state.symbols.simple_symbol(self.de.name), + typ: self.de.state.cache.kind_cache.typ(), + }, self.de.state.symbols.simple_symbol(self.variant), vec![self.de.state.cache.record(vec![], types)], ));