Skip to content

Commit

Permalink
fix: Make VmType generate correct variant constructors
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@Marwes
Marwes committed Jan 8, 2021
1 parent bbd2ee5 commit 704669e
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 23 deletions.
9 changes: 5 additions & 4 deletions base/src/types/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -860,6 +860,7 @@ impl<SpId, T> Field<SpId, T> {

pub fn ctor_with<J, Id>(
context: &mut (impl TypeContext<Id, T> + ?Sized),
enum_type: KindedIdent<Id>,
ctor_name: SpId,
elems: J,
) -> Self
Expand All @@ -868,22 +869,22 @@ impl<SpId, T> Field<SpId, T> {
J::IntoIter: DoubleEndedIterator,
T: TypePtr<Id = Id>,
{
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<J, Id>(ctor_name: SpId, elems: J) -> Self
pub fn ctor<J, Id>(enum_type: KindedIdent<Id>, ctor_name: SpId, elems: J) -> Self
where
J: IntoIterator<Item = T>,
J::IntoIter: DoubleEndedIterator,
T: TypeExt<Id = Id> + From<Type<Id, T>>,
T::Types: Default + Extend<T>,
{
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,
Expand Down
19 changes: 12 additions & 7 deletions codegen/src/arena_clone.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),*],
Expand All @@ -105,17 +105,22 @@ 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),*],
)
}}
}
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![],
)
}},
}
Expand Down
42 changes: 33 additions & 9 deletions codegen/src/vm_type.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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],
)
Expand All @@ -135,24 +148,34 @@ 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![],
)
}},
}
//----------------------------------------------------

//----------------------------------------------------
});

quote! {
_gluon_base::types::Type::variant(
vec![#(#variants),*]
Expand All @@ -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();
Expand All @@ -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,
);
Expand Down
42 changes: 41 additions & 1 deletion tests/vm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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::<OpaqueValue<&Thread, Hole>>(&vm, text);
}
23 changes: 21 additions & 2 deletions vm/src/api/typ.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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![],
));
Expand All @@ -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")],
));
Expand All @@ -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,
));
Expand All @@ -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)],
));
Expand Down

0 comments on commit 704669e

Please sign in to comment.