mattwparas / steel

An embedded scheme interpreter in Rust
Apache License 2.0
1.07k stars 50 forks source link

Add `make-string` primitive #114

Closed mattwparas closed 8 months ago

mattwparas commented 9 months ago

Missing the make-string - see the spec for details https://small.r7rs.org/attachment/r7rs.pdf

If possible, identify any tests under cogs/r5rs.scm that are skipped currently that need this primitive, and make sure they work.

See steel-core/src/primitives/strings.rs for other string primitives

duttakapil commented 9 months ago

Hi @mattwparas, thanks for creating this issue. I am going through the /strings.rs file to understand how the other string primitives are setup, and also have gone through the make-string spec definition section on r7rs.pdf. It might take me some time to finish this task, as I am still very new to Rust. Can you assign this issue to me? I will update you as I keep making some progress

gabydd commented 9 months ago

was working on this a bit, string is already implemented see here: https://github.com/mattwparas/steel/blob/4f77effec20cde870f2beae66fff8fd772bbda0c/crates/steel-core/src/primitives/strings.rs#L157-L160, I was able to implement make-string but I had to do some manual arity checking because I don't think the Range arity(which is what I assume would be used for having either 1 or 2 args is usable. Totally fine @duttakapil if you want to go ahead and work off of what I have done as I won't have time this week for this(exams). This is my initial implementation (there might be a better way to do arity that I don't know of):

#[function(name = "make-string")]
pub fn make_string(k: usize, c: RestArgs<char>) -> Result<SteelVal> {
    if c.len() > 1 {
        stop!(ArityMismatch => format!("make-string expected 1 or 2 arguments, got {}", c.len() + 1)) 
    }
    let c = c.first().unwrap_or(&'\0');
    Ok((0..k).into_iter().map(|_| c).collect::<String>().into())
}
mattwparas commented 9 months ago

Good catch on the string function already existing, missed it. And yeah, this looks fine - unfortunately the multi-arity macro primitives I have are fairly naive, but your implementation looks sane enough, just have to test it. I don't remember off hand the details of RestArgs

mattwparas commented 9 months ago

This is how you'd get this to work:

#[function(name = "make-string")]
pub fn make_string(k: usize, mut c: RestArgsIter<'_, char>) -> Result<SteelVal> {
    // If the char is there, we want to take it
    let char = c.next();

    // We want the iterator to be exhaused
    if let Some(next) = c.next() {
        stop!(ArityMismatch => format!("make-string expected 1 or 2 arguments, got an additional argument {}", next?))
    }

    let c = char.unwrap_or(Ok('\0'))?;
    Ok((0..k).into_iter().map(|_| c).collect::<String>().into())
}

In this case the RestArgs is just an iterator, so we check if it is exhausted to assert we have no more arguments

mattwparas commented 8 months ago

Fixed in #116