Update parser to new scm-slang parser (#1584)

* Prepare scheme files for new parser

* update JS version for js-slang

* proper formatting of files

* fix separate program environments across REPL eval calls

* remove logger messages from interpreter

* Enable variadic continuations for future

* Remove Infinity and NaN representation from Scheme

* Change scm-slang to follow forked version

* update scm-slang to newest parser

* resolve linting problems

* add test cases to verify proper chapter validation, decoded representation

* update scm-slang

* Move scheme-specific tests to scm-slang

* make scheme test names more obvious

* Revert "Move scheme-specific tests to scm-slang"

This reverts commit 42e184e.

* move scm-slang to dedicated alt-lang folder

* remove duplicate code between scm-slang and js-slang

* ignore alt langs coverage

---------

Co-authored-by: Martin Henz <[email protected]>

Author: Kyriel Abad

.eslintignore

@@ -1,3 +1,3 @@
 /dist/
-/src/scm-slang/
+/src/alt-langs/
 /src/py-slang/

.gitmodules

@@ -1,6 +1,6 @@
-[submodule "src/scm-slang"]
-	path = src/scm-slang
-	url = https://github.com/source-academy/scm-slang.git
+[submodule "src/alt-langs/scheme/scm-slang"]
+	path = src/alt-langs/scheme/scm-slang
+	url = https://github.com/source-academy/scm-slang
 [submodule "src/py-slang"]
 	path = src/py-slang
 	url = https://github.com/source-academy/py-slang

.prettierignore

@@ -1,3 +1,3 @@
-/src/scm-slang/
+/src/alt-langs/
 /src/py-slang/
 /src/**/__tests__/**/__snapshots__

package.json

@@ -114,7 +114,7 @@
       "/node_modules/",
       "/src/typings/",
       "/src/utils/testing.ts",
-      "/src/scm-slang",
+      "/src/alt-langs",
       "/src/py-slang/"
     ],
     "reporters": [

src/__tests__/__snapshots__/scmlib.ts.snap

@@ -0,0 +1,67 @@
+import { Node } from 'estree'
+
+import { UnassignedVariable } from '../../../errors/errors'
+import { decode, encode } from '../scm-slang/src'
+import { cons, set$45$cdr$33$ } from '../scm-slang/src/stdlib/base'
+import { dummyExpression } from '../../../utils/dummyAstCreator'
+import { decodeError, decodeValue } from '../../../parser/scheme'
+
+describe('Scheme encoder and decoder', () => {
+  it('encoder and decoder are proper inverses of one another', () => {
+    const values = [
+      'hello',
+      'hello world',
+      'scheme->JavaScript',
+      'hello world!!',
+      'true',
+      'false',
+      'null',
+      '1',
+      '😀'
+    ]
+    for (const value of values) {
+      expect(decode(encode(value))).toEqual(value)
+    }
+  })
+
+  it('decoder ignores primitives', () => {
+    const values = [1, 2, 3, true, false, null]
+    for (const value of values) {
+      expect(decodeValue(value)).toEqual(value)
+    }
+  })
+
+  it('decoder works on function toString representation', () => {
+    // Dummy function to test
+    function $65$() {}
+    expect(decodeValue($65$).toString()).toEqual(`function A() { }`)
+  })
+
+  it('decoder works on circular lists', () => {
+    function $65$() {}
+    const pair = cons($65$, 'placeholder')
+    set$45$cdr$33$(pair, pair)
+    expect(decodeValue(pair).toString()).toEqual(`function A() { },`)
+  })
+
+  it('decoder works on pairs', () => {
+    // and in doing so, works on pairs, lists, etc...
+    function $65$() {}
+    const pair = cons($65$, 'placeholder')
+    expect(decodeValue(pair).toString()).toEqual(`function A() { },placeholder`)
+  })
+
+  it('decoder works on vectors', () => {
+    // scheme doesn't support optimisation of circular vectors yet
+    function $65$() {}
+    const vector = [$65$, 2, 3]
+    expect(decodeValue(vector).toString()).toEqual(`function A() { },2,3`)
+  })
+
+  it('decodes runtime errors properly', () => {
+    const token = `😀`
+    const dummyNode: Node = dummyExpression()
+    const error = new UnassignedVariable(encode(token), dummyNode)
+    expect(decodeError(error).elaborate()).toContain(`😀`)
+  })
+})

src/__tests__/scmlib.ts

@@ -1,9 +1,12 @@
-import { parseError } from '../..'
-import { mockContext } from '../../mocks/context'
-import { Chapter } from '../../types'
-import { SchemeParser } from '../scheme'
-
-const parser = new SchemeParser(Chapter.SCHEME_1)
+import { parseError } from '../../..'
+import { mockContext } from '../../../mocks/context'
+import { Chapter } from '../../../types'
+import { SchemeParser } from '../../../parser/scheme'
+
+const parser_1 = new SchemeParser(Chapter.SCHEME_1)
+const parser_2 = new SchemeParser(Chapter.SCHEME_2)
+const parser_3 = new SchemeParser(Chapter.SCHEME_3)
+const parser_4 = new SchemeParser(Chapter.SCHEME_4)
 const parser_full = new SchemeParser(Chapter.FULL_SCHEME)
 let context = mockContext(Chapter.SCHEME_1)
 let context_full = mockContext(Chapter.FULL_SCHEME)
@@ -15,7 +18,7 @@ beforeEach(() => {
 
 describe('Scheme parser', () => {
   it('represents itself correctly', () => {
-    expect(parser.toString()).toMatchInlineSnapshot(`"SchemeParser{chapter: 1}"`)
+    expect(parser_1.toString()).toMatchInlineSnapshot(`"SchemeParser{chapter: 1}"`)
   })
 
   it('throws error if given chapter is wrong', () => {
@@ -26,13 +29,13 @@ describe('Scheme parser', () => {
 
   it('throws errors if option throwOnError is selected + parse error is encountered', () => {
     const code = `(hello))`
-    expect(() => parser.parse(code, context, undefined, true)).toThrow("Unexpected ')'")
+    expect(() => parser_1.parse(code, context, undefined, true)).toThrow("Unexpected ')'")
   })
 
   it('formats tokenizer errors correctly', () => {
     const code = `(hello))`
 
-    parser.parse(code, context)
+    parser_1.parse(code, context)
     expect(context.errors.slice(-1)[0]).toMatchObject(
       expect.objectContaining({ message: expect.stringContaining("Unexpected ')'") })
     )
@@ -41,45 +44,40 @@ describe('Scheme parser', () => {
   it('formats parser errors correctly', () => {
     const code = `(define (f x)`
 
-    parser.parse(code, context)
+    parser_1.parse(code, context)
     expect(context.errors.slice(-1)[0]).toMatchObject(
       expect.objectContaining({ message: expect.stringContaining('Unexpected EOF') })
     )
   })
 
-  it('allows usage of builtins/preludes', () => {
-    const code = `
-    (+ 1 2 3)
-    (gcd 10 15)
-    `
-
-    parser.parse(code, context)
-    expect(parseError(context.errors)).toMatchInlineSnapshot(`""`)
-  })
-
   it('allows usage of imports/modules', () => {
     const code = `(import "rune" (show heart))
       (show heart)
     `
 
-    parser.parse(code, context)
+    parser_1.parse(code, context)
     expect(parseError(context.errors)).toMatchInlineSnapshot(`""`)
   })
 
   it('disallows syntax for higher chapters', () => {
     const code = `'(1 2 3)`
 
-    parser.parse(code, context)
+    parser_1.parse(code, context)
     expect(context.errors.slice(-1)[0]).toMatchObject(
       expect.objectContaining({
         message: expect.stringContaining("Syntax ''' not allowed at Scheme §1")
       })
     )
   })
 
-  it('allows syntax for lower chapters', () => {
+  it('allows syntax for chapters of required or higher chapter', () => {
     const code = `'(1 2 3)`
 
+    // regardless of how many times we parse this code in the same context,
+    // there should be no errors in the context as long as the chapter is 2 or higher
+    parser_2.parse(code, context_full)
+    parser_3.parse(code, context_full)
+    parser_4.parse(code, context_full)
     parser_full.parse(code, context_full)
     expect(parseError(context_full.errors)).toMatchInlineSnapshot(`""`)
   })

src/alt-langs/scheme/__tests__/scheme-encode-decode.ts

@@ -11,7 +11,10 @@ Object {
   "displayResult": Array [],
   "numErrors": 0,
   "parsedErrors": "",
-  "result": 10,
+  "result": SchemeInteger {
+    "numberType": 1,
+    "value": 10n,
+  },
   "resultStatus": "finished",
   "visualiseListResult": Array [],
 }
@@ -54,7 +57,10 @@ Object {
   "displayResult": Array [],
   "numErrors": 0,
   "parsedErrors": "",
-  "result": 2,
+  "result": SchemeInteger {
+    "numberType": 1,
+    "value": 2n,
+  },
   "resultStatus": "finished",
   "visualiseListResult": Array [],
 }
@@ -92,37 +98,3 @@ Object {
   "visualiseListResult": Array [],
 }
 `;
-
-exports[`cont throws error given >1 argument: expectParsedError 1`] = `
-Object {
-  "alertResult": Array [],
-  "code": "
-    (+ 1 2 (call/cc
-              (lambda (k) (k 3 'wrongwrongwrong!)))
-            4)
-  ",
-  "displayResult": Array [],
-  "numErrors": 1,
-  "parsedErrors": "Line 3: Expected 1 arguments, but got 2.",
-  "result": undefined,
-  "resultStatus": "error",
-  "visualiseListResult": Array [],
-}
-`;
-
-exports[`cont throws error given no arguments: expectParsedError 1`] = `
-Object {
-  "alertResult": Array [],
-  "code": "
-    (+ 1 2 (call/cc
-              (lambda (k) (k)))
-            4)
-  ",
-  "displayResult": Array [],
-  "numErrors": 1,
-  "parsedErrors": "Line 3: Expected 1 arguments, but got 0.",
-  "result": undefined,
-  "resultStatus": "error",
-  "visualiseListResult": Array [],
-}
-`;

src/parser/__tests__/scheme.ts src/alt-langs/scheme/__tests__/scheme-parser.ts

@@ -13,7 +13,12 @@ test('basic call/cc works', () => {
             4)
   `,
     optionECScm
-  ).toMatchInlineSnapshot(`10`)
+  ).toMatchInlineSnapshot(`
+SchemeInteger {
+  "numberType": 1,
+  "value": 10n,
+}
+`)
 })
 
 test('call/cc can be used to escape a computation', () => {
@@ -28,7 +33,12 @@ test('call/cc can be used to escape a computation', () => {
     test
   `,
     optionECScm
-  ).toMatchInlineSnapshot(`2`)
+  ).toMatchInlineSnapshot(`
+SchemeInteger {
+  "numberType": 1,
+  "value": 2n,
+}
+`)
 })
 
 test('call/cc throws error given no arguments', () => {
@@ -52,6 +62,10 @@ test('call/cc throws error given >1 argument', () => {
   ).toMatchInlineSnapshot(`"Line 2: Expected 1 arguments, but got 2."`)
 })
 
+/*
+for now, continuations have variable arity but are unable to check for the "correct"
+number of arguments. we will omit these tests for now
+
 test('cont throws error given no arguments', () => {
   return expectParsedError(
     `
@@ -73,7 +87,7 @@ test('cont throws error given >1 argument', () => {
     optionECScm
   ).toMatchInlineSnapshot(`"Line 3: Expected 1 arguments, but got 2."`)
 })
-
+*/
 test('call/cc can be stored as a value', () => {
   return expectResult(
     `

src/alt-langs/scheme/scm-slang

@@ -50,7 +50,8 @@ export function getContinuationEnv(cn: Continuation): Environment[] {
 
 export function makeContinuation(control: Control, stash: Stash, env: Environment[]): Function {
   // Cast a function into a continuation
-  const fn: any = (x: any) => x
+  // a continuation may take any amount of arguments
+  const fn: Function = (...x: any[]) => x
   const cn: Continuation = fn as Continuation
 
   // Set the control, stash and environment

src/createContext.ts

@@ -144,7 +144,8 @@ export const genContInstr = (srcNode: Node): GenContInstr => ({
   srcNode
 })
 
-export const resumeContInstr = (srcNode: Node): ResumeContInstr => ({
+export const resumeContInstr = (numOfArgs: number, srcNode: es.Node): ResumeContInstr => ({
+  numOfArgs: numOfArgs,
   instrType: InstrType.RESUME_CONT,
   srcNode
 })

src/cse-machine/__tests__/__snapshots__/cse-machine-callcc.ts.snap

@@ -48,8 +48,10 @@
   CseError,
   EnvInstr,
   ForInstr,
+  GenContInstr,
   Instr,
   InstrType,
+  ResumeContInstr,
   UnOpInstr,
   WhileInstr
 } from './types'
@@ -248,7 +250,7 @@
  * @returns The corresponding promise.
  */
 export function CSEResultPromise(context: Context, value: Value): Promise<Result> {
   return new Promise((resolve, reject) => {
     if (value instanceof CSEBreak) {
       resolve({ status: 'suspended-cse-eval', context })
     } else if (value instanceof CseError) {
@@ -397,6 +399,42 @@
       declareFunctionsAndVariables(context, command, environment)
     }
 
+    // A strange bug occurs here when successive REPL commands are run, as they
+    // are each evaluated as separate programs. This causes the environment to be
+    // pushed multiple times.
+
+    // As such, we need to "append" the tail environment to the current environment
+    // if and only if the tail environment is a previous program environment.
+
+    const currEnv = currentEnvironment(context)
+    if (
+      currEnv &&
+      currEnv.name === 'programEnvironment' &&
+      currEnv.tail &&
+      currEnv.tail.name === 'programEnvironment'
+    ) {
+      // we need to take that tail environment and append its items to the current environment
+      const oldEnv = currEnv.tail
+
+      // separate the tail environment from the environments list
+      currEnv.tail = oldEnv.tail
+
+      // we will recycle the old environment's item list
+      // add the items from the current environment to the tail environment
+      // this is fine, especially as the older program will never
+      // need to use the old environment's items again
+      for (const key in currEnv.head) {
+        oldEnv.head[key] = currEnv.head[key]
+      }
+
+      // set the current environment to the old one
+      // this will work across successive programs as well
+
+      // this will also allow continuations to read newer program
+      // values from their "outdated" program environment
+      currEnv.head = oldEnv.head
+    }
+
     if (command.body.length == 1) {
       // If program only consists of one statement, evaluate it immediately
       const next = command.body[0]
@@ -450,7 +488,7 @@
     command: es.WhileStatement,
     context: Context,
     control: Control,
     stash: Stash
   ) {
     if (hasBreakStatement(command.body as es.BlockStatement)) {
       control.push(instr.breakMarkerInstr(command))
@@ -533,7 +571,7 @@
     command: es.IfStatement,
     context: Context,
     control: Control,
     stash: Stash
   ) {
     control.push(...reduceConditional(command))
   },
@@ -606,7 +644,7 @@
     command: es.ContinueStatement,
     context: Context,
     control: Control,
     stash: Stash
   ) {
     control.push(instr.contInstr(command))
   },
@@ -615,7 +653,7 @@
     command: es.BreakStatement,
     context: Context,
     control: Control,
     stash: Stash
   ) {
     control.push(instr.breakInstr(command))
   },
@@ -662,7 +700,7 @@
     command: es.MemberExpression,
     context: Context,
     control: Control,
     stash: Stash
   ) {
     control.push(instr.arrAccInstr(command))
     control.push(command.property)
@@ -673,7 +711,7 @@
     command: es.ConditionalExpression,
     context: Context,
     control: Control,
     stash: Stash
   ) {
     control.push(...reduceConditional(command))
   },
@@ -744,7 +782,7 @@
    * Instructions
    */
 
   [InstrType.RESET]: function (command: Instr, context: Context, control: Control, stash: Stash) {
     // Keep pushing reset instructions until marker is found.
     const cmdNext: ControlItem | undefined = control.pop()
     if (cmdNext && (isNode(cmdNext) || cmdNext.instrType !== InstrType.MARKER)) {
@@ -914,17 +952,16 @@
       // Check for number of arguments mismatch error
       checkNumberOfArguments(context, func, args, command.srcNode)
 
-      // A continuation is always given a single argument
-      const expression: Value = args[0]
-
       const dummyContCallExpression = makeDummyContCallExpression('f', 'cont')
 
       // Restore the state of the stash,
       // but replace the function application instruction with
       // a resume continuation instruction
       stash.push(func)
-      stash.push(expression)
-      control.push(instr.resumeContInstr(dummyContCallExpression))
+      // we need to push the arguments back onto the stash
+      // as well
+      stash.push(...args)
+      control.push(instr.resumeContInstr(command.numOfArgs, dummyContCallExpression))
       return
     }
 
@@ -1075,7 +1112,7 @@
     command: Instr,
     context: Context,
     control: Control,
     stash: Stash
   ) {
     const next = control.pop() as ControlItem
     if (isInstr(next) && next.instrType == InstrType.CONTINUE_MARKER) {
@@ -1091,7 +1128,7 @@
 
   [InstrType.CONTINUE_MARKER]: function () {},
 
   [InstrType.BREAK]: function (command: Instr, context: Context, control: Control, stash: Stash) {
     const next = control.pop() as ControlItem
     if (isInstr(next) && next.instrType == InstrType.BREAK_MARKER) {
       // Encountered break mark, stop popping
@@ -1107,7 +1144,7 @@
   [InstrType.BREAK_MARKER]: function () {},
 
   [InstrType.GENERATE_CONT]: function (
-    _command: Instr,
+    _command: GenContInstr,
     context: Context,
     control: Control,
     stash: Stash
@@ -1128,12 +1165,16 @@
   },
 
   [InstrType.RESUME_CONT]: function (
-    _command: Instr,
+    command: ResumeContInstr,
     context: Context,
     control: Control,
     stash: Stash
   ) {
-    const expression = stash.pop()
+    // pop the arguments
+    const args: Value[] = []
+    for (let i = 0; i < command.numOfArgs; i++) {
+      args.unshift(stash.pop())
+    }
     const cn: Continuation = stash.pop() as Continuation
 
     const contControl = getContinuationControl(cn)
@@ -1144,10 +1185,10 @@
     control.setTo(contControl)
     stash.setTo(contStash)
 
-    // Push the expression given to the continuation onto the stash
-    stash.push(expression)
+    // Push the arguments given to the continuation back onto the stash
+    stash.push(...args)
 
-    // Restore the environment pointer to that of the continuation
+    // Restore the environment pointer to that of the continuation's environment
     context.runtime.environments = contEnv
   }
 }

src/cse-machine/__tests__/cse-machine-callcc.ts

@@ -77,7 +77,9 @@ export interface ArrLitInstr extends BaseInstr {
 
 export type GenContInstr = BaseInstr
 
-export type ResumeContInstr = BaseInstr
+export interface ResumeContInstr extends BaseInstr {
+  numOfArgs: number
+}
 
 export type Instr =
   | BaseInstr

src/cse-machine/continuations.ts

@@ -7,6 +7,7 @@ import { RuntimeSourceError } from '../errors/runtimeSourceError'
 import Closure from '../interpreter/closure'
 import { Environment, Frame, Node, StatementSequence, Value } from '../types'
 import * as ast from '../utils/astCreator'
+import { isContinuation } from './continuations'
 import * as instr from './instrCreator'
 import { Control } from './interpreter'
 import { AppInstr, AssmtInstr, ControlItem, Instr, InstrType } from './types'
@@ -490,6 +491,12 @@ export const checkNumberOfArguments = (
         )
       )
     }
+  } else if (isContinuation(callee)) {
+    // Continuations have variadic arguments,
+    // and so we can let it pass
+    // in future, if we can somehow check the number of arguments
+    // expected by the continuation, we can add a check here.
+    return undefined
   } else {
     // Pre-built functions
     const hasVarArgs = callee.minArgsNeeded != undefined

src/cse-machine/instrCreator.ts

@@ -1,12 +1,23 @@
-import { Node, Program } from 'estree'
+import { Program } from 'estree'
 
-import { decode, encode, schemeParse } from '../../scm-slang/src'
-import { Pair } from '../../scm-slang/src/stdlib/source-scheme-library'
+import { decode, schemeParse } from '../../alt-langs/scheme/scm-slang/src'
+import {
+  car,
+  cdr,
+  circular$45$list$63$,
+  cons,
+  last$45$pair,
+  list$45$tail,
+  pair$63$,
+  procedure$63$,
+  set$45$cdr$33$,
+  vector$63$
+} from '../../alt-langs/scheme/scm-slang/src/stdlib/base'
+import { List, Pair } from '../../stdlib/list'
 import { Chapter, Context, ErrorType, SourceError } from '../../types'
 import { FatalSyntaxError } from '../errors'
 import { AcornOptions, Parser } from '../types'
 import { positionToSourceLocation } from '../utils'
-const walk = require('acorn-walk')
 
 export class SchemeParser implements Parser<AcornOptions> {
   private chapter: number
@@ -21,10 +32,8 @@ export class SchemeParser implements Parser<AcornOptions> {
   ): Program | null {
     try {
       // parse the scheme code
-      const estree = schemeParse(programStr, this.chapter)
-      // walk the estree and encode all identifiers
-      encodeTree(estree)
-      return estree as unknown as Program
+      const estree = schemeParse(programStr, this.chapter, true)
+      return estree as Program
     } catch (error) {
       if (error instanceof SyntaxError) {
         error = new FatalSyntaxError(positionToSourceLocation((error as any).loc), error.toString())
@@ -63,15 +72,6 @@ function getSchemeChapter(chapter: Chapter): number {
   }
 }
 
-export function encodeTree(tree: Program): Program {
-  walk.full(tree, (node: Node) => {
-    if (node.type === 'Identifier') {
-      node.name = encode(node.name)
-    }
-  })
-  return tree
-}
-
 function decodeString(str: string): string {
   return str.replace(/\$scheme_[\w$]+|\$\d+\$/g, match => {
     return decode(match)
@@ -80,18 +80,58 @@ function decodeString(str: string): string {
 
 // Given any value, decode it if and
 // only if an encoded value may exist in it.
+// this function is used to accurately display
+// values in the REPL.
 export function decodeValue(x: any): any {
-  // In future: add support for decoding vectors.
-  if (x instanceof Pair) {
+  // helper version of list_tail that assumes non-null return value
+  function list_tail(xs: List, i: number): List {
+    if (i === 0) {
+      return xs
+    } else {
+      return list_tail(list$45$tail(xs), i - 1)
+    }
+  }
+
+  if (circular$45$list$63$(x)) {
     // May contain encoded strings.
-    return new Pair(decodeValue(x.car), decodeValue(x.cdr))
-  } else if (x instanceof Array) {
+    let circular_pair_index = -1
+    const all_pairs: Pair<any, any>[] = []
+
+    // iterate through all pairs in the list until we find the circular pair
+    let current = x
+    while (current !== null) {
+      if (all_pairs.includes(current)) {
+        circular_pair_index = all_pairs.indexOf(current)
+        break
+      }
+      all_pairs.push(current)
+      current = cdr(current)
+    }
+    x
+    // assemble a new list using the elements in all_pairs
+    let new_list = null
+    for (let i = all_pairs.length - 1; i >= 0; i--) {
+      new_list = cons(decodeValue(car(all_pairs[i])), new_list)
+    }
+
+    // finally we can set the last cdr of the new list to the circular-pair itself
+
+    const circular_pair = list_tail(new_list, circular_pair_index)
+    set$45$cdr$33$(last$45$pair(new_list), circular_pair)
+    return new_list
+  } else if (pair$63$(x)) {
+    // May contain encoded strings.
+    return cons(decodeValue(car(x)), decodeValue(cdr(x)))
+  } else if (vector$63$(x)) {
     // May contain encoded strings.
     return x.map(decodeValue)
-  } else if (x instanceof Function) {
+  } else if (procedure$63$(x)) {
+    // copy x to avoid modifying the original object
+    const newX = { ...x }
     const newString = decodeString(x.toString())
-    x.toString = () => newString
-    return x
+    // change the toString method to return the decoded string
+    newX.toString = () => newString
+    return newX
   } else {
     // string, number, boolean, null, undefined
     // no need to decode.

src/cse-machine/interpreter.ts

@@ -3,7 +3,7 @@
     "outDir": "./dist",
     "module": "commonjs",
     "declaration": true,
-    "target": "es2016",
+    "target": "es2020",
     "lib": [
       "es2021.string",
       "es2018",