Sounds: long-established demo no longer working properly
Closed this issue · 7 comments
Expected Behavior
Bohemian rapsody cover should be heard when playing
https://share.sourceacademy.org/g9n0m
using Source §2 or Source §2 native
Current Behavior
Something else is heard that has some of the harmonies of Bohemian rapsody, but sounds very different.
Failure Information (for bugs)
Steps to Reproduce
Run
https://share.sourceacademy.org/g9n0m
Context
MacBook Pro, x86, MacOS Monterey
i think the issue might be in js-slang repo under src/stdlib/list.ts
accumulate
function, related to the changes made in PR source-academy/js-slang#1388
the current implementation of accumulate
reverses the result and modules is making use of the stdlib/list
in js-slang
export function accumulate<T, U>(acc: (each: T, result: U) => any, init: U, xs: List): U {
const recurser = (xs: List, result: U): U => {
if (is_null(xs)) return result
return recurser(tail(xs), acc(head(xs), result))
}
return recurser(xs, init)
}
Hmm this one's on me. I'll fix this and incorporate some tests with my latest js-slang PR
OK. Note that the accumulate function in Source Academy uses continuation passing style, to make sure it doesn't lead to JS-runtime-stack overflow:
function accumulate(f, initial, xs) {
return $accumulate(f, initial, xs, x => x);
}
function $accumulate(f, initial, xs, cont) {
return is_null(xs) ? cont(initial) : $accumulate(f, initial, tail(xs), x => cont(f(head(xs), x)));
}
A note on the list library: We have two versions:
(1) https://github.com/source-academy/js-slang/blob/master/src/stdlib/list.ts
That's the version available for import in modules. It is the version that has the bug. Is this version used elsewhere?
(2) https://github.com/source-academy/js-slang/blob/master/src/stdlib/list.prelude.ts
This is the version that is used in Source Academy to implement the list functions. This is done by 'eval'ing the string that has the source code for the list functions. One reason for this is that the display values of the functions are nice clean JavaScript without type declarations.
I think having two versions is acceptable because they seem to serve different purposes. Maybe the version (1) isn't covered by regression testing, which means the bug slipped through.
[BTW, there is a third version:
(3) https://github.com/source-academy/js-slang/blob/master/docs/lib/list.js
This version is a variant of the prelude version above and is used to generate the documentation of the list library.]
Would the following implementation work?
export function accumulate<T, U>(op: (each: T, result: U) => any, initial: U, sequence: List): U {
// Use CPS to prevent stack overflow
function $accumulate(f: typeof op, xs: typeof sequence, cont: (each: U) => U): U {
return is_null(xs) ? cont(initial) : $accumulate(f, tail(xs), x => cont(f(head(xs), x)));
}
return $accumulate(op, sequence, x => x);
}
With this implementation the sounds demo works as expected
yes, that's correct. You can include this in a PR.
Prof @martin-henz, we need to bump js-slang to fix this fully. Otherwise, this issue is closed via source-academy/js-slang#1463.