CSE: Fix buggy stream function checking (#1657)

* Fix wrongly adding environment property to built-in functions

* Fix condition for stream function

* bumping version

---------

Co-authored-by: henz <[email protected]>

package.json

@@ -1,6 +1,6 @@
 {
   "name": "js-slang",
-  "version": "1.0.63",
+  "version": "1.0.64",
   "license": "Apache-2.0",
   "description": "Javascript-based implementations of Source, written in Typescript",
   "keywords": [

src/cse-machine/interpreter.ts

@@ -7,7 +7,7 @@
 
 /* tslint:disable:max-classes-per-file */
 import * as es from 'estree'
-import { isArray, isFunction, reverse } from 'lodash'
+import { isArray, reverse } from 'lodash'
 
 import { IOptions } from '..'
 import { UNKNOWN_LOCATION } from '../constants'
@@ -75,6 +75,7 @@ import {
   isInstr,
   isNode,
   isSimpleFunction,
+  isStreamFn,
   popEnvironment,
   pushEnvironment,
   reduceConditional,
@@ -987,6 +988,18 @@ const cmdEvaluators: { [type: string]: CmdEvaluator } = {
     try {
       const result = func(...args)
 
+      if (isStreamFn(func, result)) {
+        // This is a special case for the `stream` built-in function, since it returns pairs
+        // whose last element is a function. The CSE machine on the frontend will still draw
+        // these functions like closures, and the tail of the "closures" will need to point
+        // to where `stream` was called.
+        //
+        // TODO: remove this condition if `stream` becomes a pre-defined function
+        Object.defineProperties(result[1], {
+          environment: { value: currentEnvironment(context), writable: true }
+        })
+      }
+
       // Recursively adds `environment` and `id` properties to any arrays created,
       // and also adds them to the heap starting from the arrays that are more deeply nested.
       const attachEnvToResult = (value: any) => {
@@ -997,16 +1010,6 @@ const cmdEvaluators: { [type: string]: CmdEvaluator } = {
             attachEnvToResult(item)
           }
           handleArrayCreation(context, value)
-        } else if (isFunction(value) && !{}.hasOwnProperty.call(value, 'environment')) {
-          // This is a special case for the `stream` built-in function, since it returns pairs
-          // whose last element is a function. The CSE machine on the frontend will still draw
-          // these functions like closures, and the tail of the "closures" will need to point
-          // to where `stream` was called.
-          //
-          // TODO: remove this condition if `stream` becomes a pre-defined function
-          Object.defineProperties(value, {
-            environment: { value: currentEnvironment(context), writable: true }
-          })
         }
       }
       attachEnvToResult(result)

src/cse-machine/utils.ts

@@ -1,5 +1,5 @@
 import * as es from 'estree'
-import { isArray } from 'lodash'
+import { isArray, isFunction } from 'lodash'
 
 import { Context } from '..'
 import * as errors from '../errors/errors'
@@ -103,6 +103,9 @@ export const uniqueId = (context: Context): string => {
   return `${context.runtime.objectCount++}`
 }
 
+/**
+ * Returns whether `item` is an array with `id` and `environment` properties already attached.
+ */
 export const isEnvArray = (item: any): item is EnvArray => {
   return (
     isArray(item) &&
@@ -111,6 +114,20 @@ export const isEnvArray = (item: any): item is EnvArray => {
   )
 }
 
+/**
+ * Returns whether `item` is a non-closure function that returns a stream.
+ * If the function has been called already and we have the result, we can
+ * pass it in here as a 2nd argument for stronger checking
+ */
+export const isStreamFn = (item: any, result?: any): result is [any, () => any] => {
+  if (result == null || !isArray(result) || result.length !== 2) return false
+  return (
+    isFunction(item) &&
+    !(item instanceof Closure) &&
+    (item.name === 'stream' || {}.hasOwnProperty.call(item, 'environment'))
+  )
+}
+
 /**
  * Adds the properties `id` and `environment` to the given array, and adds the array to the
  * current environment's heap. Adds the array to the heap of `envOverride` instead if it's defined.