Skip to content

Commit

Permalink
fix panic in language server
Browse files Browse the repository at this point in the history
  • Loading branch information
mattwparas committed Dec 23, 2023
1 parent a63c49d commit bfc5a8d
Show file tree
Hide file tree
Showing 11 changed files with 116 additions and 127 deletions.
7 changes: 7 additions & 0 deletions crates/steel-core/src/primitives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,13 @@ impl FromSteelVal for SteelString {
}
}

// impl<'a, T: AsRefSteelVal> PrimitiveAsRef<'a> for &'a T {
// #[inline(always)]
// fn primitive_as_ref(val: &'a SteelVal) -> crate::rvals::Result<Self> {
// AsRefSteelVal::as_ref(val, &mut <T as AsRefSteelVal>::Nursery::default())
// }
// }

impl<'a> PrimitiveAsRef<'a> for &'a Gc<RefCell<SteelVal>> {
#[inline(always)]
fn primitive_as_ref(val: &'a SteelVal) -> crate::rvals::Result<Self> {
Expand Down
56 changes: 17 additions & 39 deletions crates/steel-core/src/primitives/hashmaps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ declare_const_ref_functions!(
HM_CONSTRUCT => hm_construct,
HM_INSERT => steel_hash_insert,
HM_GET => steel_hash_ref,
HM_CLEAR => clear,
HM_EMPTY => hm_empty,
HM_UNION => hm_union,
);

pub(crate) fn hashmap_module() -> BuiltInModule {
Expand All @@ -34,9 +32,9 @@ pub(crate) fn hashmap_module() -> BuiltInModule {
.register_native_fn_definition(KEYS_TO_VECTOR_DEFINITION)
.register_native_fn_definition(VALUES_TO_LIST_DEFINITION)
.register_native_fn_definition(VALUES_TO_VECTOR_DEFINITION)
.register_value("hash-clear", HM_CLEAR)
.register_native_fn_definition(CLEAR_DEFINITION)
.register_value("hash-empty?", HM_EMPTY)
.register_value("hash-union", HM_UNION);
.register_native_fn_definition(HM_UNION_DEFINITION);
module
}

Expand Down Expand Up @@ -256,30 +254,21 @@ pub fn values_to_list(hashmap: &Gc<HashMap<SteelVal, SteelVal>>) -> Result<Steel
Ok(SteelVal::ListV(hashmap.values().cloned().collect()))
}

#[steel_derive::function(name = "hm-keys->vector")]
#[steel_derive::function(name = "hash-keys->vector")]
pub fn keys_to_vector(hashmap: &Gc<HashMap<SteelVal, SteelVal>>) -> Result<SteelVal> {
VectorOperations::vec_construct_iter_normal(hashmap.keys().cloned())
}

#[steel_derive::function(name = "hm-values->vector")]
#[steel_derive::function(name = "hash-values->vector")]
pub fn values_to_vector(hashmap: &Gc<HashMap<SteelVal, SteelVal>>) -> Result<SteelVal> {
VectorOperations::vec_construct_iter_normal(hashmap.values().cloned())
}

pub fn clear(args: &[SteelVal]) -> Result<SteelVal> {
if args.len() != 1 {
stop!(ArityMismatch => "hm-clear takes 1 argument")
}

let hashmap = &args[0];

if let SteelVal::HashMapV(hm) = hashmap {
let mut hm = hm.0.unwrap();
hm.clear();
Ok(SteelVal::HashMapV(Gc::new(hm).into()))
} else {
stop!(TypeMismatch => "hm-clear takes a hashmap")
}
#[steel_derive::function(name = "hash-clear")]
pub fn clear(hashmap: &Gc<HashMap<SteelVal, SteelVal>>) -> Result<SteelVal> {
let mut hm = hashmap.unwrap();
hm.clear();
Ok(SteelVal::HashMapV(Gc::new(hm).into()))
}

pub fn hm_empty(args: &[SteelVal]) -> Result<SteelVal> {
Expand All @@ -296,25 +285,14 @@ pub fn hm_empty(args: &[SteelVal]) -> Result<SteelVal> {
}
}

pub fn hm_union(args: &[SteelVal]) -> Result<SteelVal> {
if args.len() != 2 {
stop!(ArityMismatch => "hash-union takes 2 arguments")
}

let left = &args[0];
let right = &args[1];

if let SteelVal::HashMapV(hml) = left {
if let SteelVal::HashMapV(hmr) = right {
let hml = hml.0.unwrap();
let hmr = hmr.0.unwrap();
Ok(SteelVal::HashMapV(Gc::new(hml.union(hmr)).into()))
} else {
stop!(TypeMismatch => "hash-union takes a hashmap, found {}", right)
}
} else {
stop!(TypeMismatch => "hash-union takes a hashmap, found: {}", left)
}
#[steel_derive::function(name = "hash-union")]
pub fn hm_union(
hml: &Gc<HashMap<SteelVal, SteelVal>>,
hmr: &Gc<HashMap<SteelVal, SteelVal>>,
) -> Result<SteelVal> {
let hml = hml.unwrap();
let hmr = hmr.unwrap();
Ok(SteelVal::HashMapV(Gc::new(hml.union(hmr)).into()))
}

#[cfg(test)]
Expand Down
35 changes: 25 additions & 10 deletions crates/steel-core/src/rvals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,17 +441,17 @@ pub trait AsRefSteelValFromRef: Sized {
fn as_ref_from_ref<'a>(val: &'a SteelVal) -> crate::rvals::Result<&'a Self>;
}

impl AsRefSteelVal for List<SteelVal> {
type Nursery = ();
// impl AsRefSteelVal for List<SteelVal> {
// type Nursery = ();

fn as_ref<'b, 'a: 'b>(val: &'a SteelVal, _nursery: &mut ()) -> Result<SRef<'b, Self>> {
if let SteelVal::ListV(l) = val {
Ok(SRef::Temporary(l))
} else {
stop!(TypeMismatch => "Value cannot be referenced as a list")
}
}
}
// fn as_ref<'b, 'a: 'b>(val: &'a SteelVal, _nursery: &mut ()) -> Result<SRef<'b, Self>> {
// if let SteelVal::ListV(l) = val {
// Ok(SRef::Temporary(l))
// } else {
// stop!(TypeMismatch => "Value cannot be referenced as a list")
// }
// }
// }

impl AsRefSteelVal for UserDefinedStruct {
type Nursery = ();
Expand Down Expand Up @@ -1112,6 +1112,21 @@ impl From<Gc<im_rc::HashSet<SteelVal>>> for SteelHashSet {
}
}

pub enum TypeKind {
Any,
Bool,
Num,
Int,
Char,
Vector(Box<TypeKind>),
Void,
String,
Function,
HashMap(Box<TypeKind>, Box<TypeKind>),
HashSet(Box<TypeKind>),
List(Box<TypeKind>),
}

/// A value as represented in the runtime.
#[derive(Clone)]
pub enum SteelVal {
Expand Down
47 changes: 2 additions & 45 deletions crates/steel-core/src/steel_vm/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{borrow::Cow, cell::RefCell, rc::Rc, sync::Arc};
use crate::{
containers::RegisterValue,
parser::{ast::ExprKind, interner::InternedString, parser::SyntaxObject, tokens::TokenType},
rvals::{Custom, FromSteelVal, FunctionSignature, IntoSteelVal, Result, SteelVal},
rvals::{Custom, FromSteelVal, FunctionSignature, IntoSteelVal, Result, SteelVal, TypeKind},
values::functions::BoxedDynFunction,
};
use im_rc::HashMap;
Expand Down Expand Up @@ -125,6 +125,7 @@ pub struct NativeFunctionDefinition {
pub arity: Arity,
pub doc: Option<MarkdownDoc<'static>>,
pub is_const: bool,
pub signature: Option<(&'static [TypeKind], TypeKind)>,
}

impl BuiltInModuleRepr {
Expand All @@ -138,37 +139,6 @@ impl BuiltInModuleRepr {
}
}

// pub fn register_native_fn(
// &mut self,
// name: &'static str,
// func: fn(&[SteelVal]) -> Result<SteelVal>,
// arity: Arity,
// ) -> &mut Self {
// // Just automatically add it to the function pointer table to help out with searching
// self.add_to_fn_ptr_table(func, FunctionSignatureMetadata::new(name, arity, false));
// self.register_value(name, SteelVal::FuncV(func))
// }

// pub fn register_native_fn_definition(
// &mut self,
// definition: NativeFunctionDefinition,
// ) -> &mut Self {
// self.add_to_fn_ptr_table(
// definition.func,
// FunctionSignatureMetadata::new(definition.name, definition.arity, definition.is_const),
// );
// if let Some(doc) = definition.doc {
// self.register_doc(definition.name, doc);
// }
// self.register_value(definition.name, SteelVal::FuncV(definition.func));
// self
// }

// pub fn check_compatibility(self: &BuiltInModule) -> bool {
// // self.version == env!("CARGO_PKG_VERSION")
// true
// }

pub fn contains(&self, ident: &str) -> bool {
self.values.contains_key(ident)
}
Expand Down Expand Up @@ -443,19 +413,6 @@ impl BuiltInModule {
value: FunctionSignature,
data: FunctionSignatureMetadata,
) -> &mut Self {
// // Store this in a globally accessible place for printing
// FUNCTION_TABLE.with(|table| {
// table
// .borrow_mut()
// .insert(value as *const FunctionSignature, data)
// });

// // Probably don't need to store it in both places?
// self.fn_ptr_table
// .insert(value as *const FunctionSignature, data);

// self

self.module.borrow_mut().add_to_fn_ptr_table(value, data);

self
Expand Down
30 changes: 15 additions & 15 deletions crates/steel-core/src/steel_vm/vm/threads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,24 @@ pub fn closure_into_serializable(
}
}

struct MovableThread {
constants: Vec<SerializableSteelVal>,
global_env: Vec<SerializableSteelVal>,
function_interner: MovableFunctionInterner,
runtime_options: RunTimeOptions,
}

struct MovableFunctionInterner {
closure_interner: fxhash::FxHashMap<usize, SerializedLambda>,
pure_function_interner: fxhash::FxHashMap<usize, SerializedLambda>,
spans: fxhash::FxHashMap<usize, Vec<Span>>,
instructions: fxhash::FxHashMap<usize, Vec<DenseInstruction>>,
}

/// This will naively deep clone the environment, by attempting to translate every value into a `SerializableSteelVal`
/// While this does work, it does result in a fairly hefty deep clone of the environment. It does _not_ smartly attempt
/// to keep track of what values this function could touch - rather it assumes every value is possible to be touched
/// by the child thread. In addition - closures which capture mutable variables are unable to be moved across threads.
/// Only pure functions and/or functions which capture immutable can be moved.
/// by the child thread.
fn spawn_thread_result(ctx: &mut VmCore, args: &[SteelVal]) -> Result<SteelVal> {
use crate::rvals::SerializableSteelVal;

Expand All @@ -127,19 +140,6 @@ fn spawn_thread_result(ctx: &mut VmCore, args: &[SteelVal]) -> Result<SteelVal>
// global env - This we can do (hopefully) lazily. Only clone the values that actually
// get referenced. We can also just straight up reject any closures that cannot be moved
// across threads
struct MovableThread {
constants: Vec<SerializableSteelVal>,
global_env: Vec<SerializableSteelVal>,
function_interner: MovableFunctionInterner,
runtime_options: RunTimeOptions,
}

struct MovableFunctionInterner {
closure_interner: fxhash::FxHashMap<usize, SerializedLambda>,
pure_function_interner: fxhash::FxHashMap<usize, SerializedLambda>,
spans: fxhash::FxHashMap<usize, Vec<Span>>,
instructions: fxhash::FxHashMap<usize, Vec<DenseInstruction>>,
}

if args.len() != 1 {
stop!(ArityMismatch => "spawn-thread! accepts one argument, found: {}", args.len())
Expand Down
1 change: 1 addition & 0 deletions crates/steel-core/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ test_harness_success! {
define_normal,
dfs,
dll,
docs,
empty,
ellipses,
fib,
Expand Down
6 changes: 6 additions & 0 deletions crates/steel-core/src/tests/success/docs.scm
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
;;@doc
;; This is a function that does something
(define (foo x)
(list 10 20 30 40 x))

(assert! (equal? "This is a function that does something\n" foo__doc__))
28 changes: 25 additions & 3 deletions crates/steel-derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ pub fn native(
arity: crate::steel_vm::builtin::Arity::#arity_number,
doc: Some(crate::steel_vm::builtin::MarkdownDoc(#doc)),
is_const: #is_const,
signature: None,
};
}
} else {
Expand All @@ -216,6 +217,7 @@ pub fn native(
arity: crate::steel_vm::builtin::Arity::#arity_number,
doc: None,
is_const: #is_const,
signature: None,
};
}
};
Expand Down Expand Up @@ -305,6 +307,9 @@ pub fn function(

let mut rest_arg_generic_inner_type = false;

// let mut argument_signatures: Vec<&'static str> = Vec::new();
// let mut return_type = "void";

for (i, arg) in sign.inputs.iter().enumerate() {
if let FnArg::Typed(pat_ty) = arg.clone() {
if let Type::Path(p) = pat_ty.ty.as_ref() {
Expand All @@ -328,15 +333,30 @@ pub fn function(
}
}

/*
TODO: Attempt to bake the type information into the native
function definition. This can give a lot more help to the optimizer
and also the LSP if we have the types for every built in definition
when we make it.
// Attempt to calculate the function signature
match pat_ty.ty.clone().into_token_stream().to_string().as_str() {
"char" => argument_signatures.push("char"),
"bool" => argument_signatures.push("bool"),
"f64" => argument_signatures.push("f64"),
"isize" => argument_signatures.push("isize"),
"SteelString" => argument_signatures.push("string"),
"&SteelString" => argument_signatures.push("string"),
_ => argument_signatures.push("any"),
}
*/

type_vec.push(pat_ty.ty);
}
}

// panic!("{:?}", type_vec);

let arity_number = type_vec.len();

// TODO: Might be able to handle types with lifetimes here, both coming in and leaving
let conversion_functions = type_vec.clone().into_iter().map(|x| {
if let Type::Reference(_) = *x {
quote! { primitive_as_ref }
Expand Down Expand Up @@ -378,6 +398,7 @@ pub fn function(
arity: crate::steel_vm::builtin::Arity::#arity_exactness(#arity_number),
doc: Some(crate::steel_vm::builtin::MarkdownDoc(#doc)),
is_const: #is_const,
signature: None,
};
}
} else {
Expand All @@ -388,6 +409,7 @@ pub fn function(
arity: crate::steel_vm::builtin::Arity::#arity_exactness(#arity_number),
doc: None,
is_const: #is_const,
signature: None
};
}
};
Expand Down
2 changes: 1 addition & 1 deletion crates/steel-language-server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ tower-lsp = { version = "0.20.0", features = ["proposed"] }
serde = { version = "1.0.152", features = ["derive"] }
dashmap = "5.1.0"
log = "0.4.17"
steel-core = { path = "../steel-core", version = "0.5.0" }
steel-core = { path = "../steel-core", version = "0.5.0", features = ["modules", "sqlite"] }
once_cell = "1.18.0"
6 changes: 5 additions & 1 deletion crates/steel-language-server/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,11 @@ impl<'a, 'b> VisitorMutUnitRef<'a> for StaticCallSiteArityChecker<'a, 'b> {
{
// If it is something that... doesn't happen in the file we're currently
// looking at - we probably shouldn't report it there?
if !l.first_ident().unwrap().resolve().starts_with("mangler") {
if !l
.first_ident()
.map(|x| x.resolve().starts_with("mangler"))
.unwrap_or(false)
{
if let Some(info) = self.context.analysis.get_identifier(function_call_ident) {
if info.builtin {
let found_arity = self
Expand Down
Loading

0 comments on commit bfc5a8d

Please sign in to comment.