Skip to content

[RFC]: add support for eager evaluation of side-effect free code in the REPL #2073

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
3 tasks done
kgryte opened this issue Mar 27, 2024 · 22 comments
Open
3 tasks done
Labels
difficulty: 4 Likely to be moderately difficult. Enhancement Issue or pull request for enhancing existing functionality. JavaScript Issue involves or relates to JavaScript. Needs Discussion Needs further discussion. priority: Normal Normal priority concern or feature request. REPL Issue or pull request specific to the project REPL. RFC Request for comments. Feature requests and proposed changes.

Comments

@kgryte
Copy link
Member

kgryte commented Mar 27, 2024

Description

This RFC proposes adding support for eager evaluation of side-effect free code in the REPL. E.g.,

In [1]: [ 'foo', 'bar', 'beep' ].sort()<|>
[ 'bar', 'beep', 'foo' ]

would trigger a preview below the input command, where <|> represents the cursor. Upon hitting ENTER, the command would execute with the result displayed after the output prompt, as normal.

This feature is now implemented in browser devtools, such as Chrome (see https://developer.chrome.com/blog/new-in-devtools-68/#eagerevaluation).

The tricky bit with implementing this is detecting side effects. And we'd likely want to avoid eagerly evaluating things like HTTP requests. Additionally, we'd like want to exclude things which are not idempotent (e.g., PRNGs, Date constructor, etc).

There is also the concern as to how to handle eager evaluation of computationally expensive commands or commands returning large outputs.

As this feature may not be desired by all users, we should presumably support an option for enabling/disabling this on REPL instantiation. And we should disable eager evaluation when not in TTY mode.

Related Issues

Questions

No.

Other

Resources:

Checklist

  • I have read and understood the Code of Conduct.
  • Searched for existing issues and pull requests.
  • The issue name begins with RFC:.
@kgryte kgryte added Enhancement Issue or pull request for enhancing existing functionality. RFC Request for comments. Feature requests and proposed changes. Needs Discussion Needs further discussion. REPL Issue or pull request specific to the project REPL. priority: Normal Normal priority concern or feature request. JavaScript Issue involves or relates to JavaScript. difficulty: 4 Likely to be moderately difficult. labels Mar 27, 2024
@Vinit-Pandit
Copy link
Contributor

@kgryte,
Please check weather i am on right track or not :

we need to trigger the processLine method (similar to the one in Multiline Handler) on every key enter in the terminal. If no errors occur during line processing and compilation, the output should be displayed as a ghost line after execution using either runInContext or runInThisContext.

Is this the correct high-level idea?

@Vinit-Pandit
Copy link
Contributor

Vinit-Pandit commented Nov 30, 2024

Or we need method which tell us weather the command is complete or not . cases like

  1. weather enter keyword is complete keyword from the context or not
  2. command not end with ','
    and others

and on the bases of this method we can go forward to process line and show the ghost output

@Snehil-Shah
Copy link
Member

Snehil-Shah commented Nov 30, 2024

@MeastroZI as mentioned in the RFC, we should not be evaluating things which might trigger stuff like HTTP requests or any other stuff that can have side effects (ref). Hence, directly trying to execute the input is not the way.

@Vinit-Pandit
Copy link
Contributor

Vinit-Pandit commented Nov 30, 2024

@Snehil-Shah so we need to check that weather the input is pure have no side effect , then after that we have to go further ? ,

And what about the context should we have to use the other context ? so it not pollute the global one as a side effect ?

@Vinit-Pandit
Copy link
Contributor

so we need to check that weather the input is pure have no side effect

And have to implement something similar to the constantinople or to use it ?

@Snehil-Shah
Copy link
Member

Snehil-Shah commented Nov 30, 2024

And what about the context should we have to use the other context ? so it not pollute the global one as a side effect ?

I think we would have to consider the global context. In devtools console, when you define a global variable, let's say a string. And in a later statement, you run a "pure" function on that string, it still eagerly evaluates. So, no, the context shouldn't be confined to that very input.

I would suggest playing around with the chrome devtools console to get an idea of expected behavior. You might even find patterns that can help you understand the underlying implementation.

@Snehil-Shah
Copy link
Member

And have to implement something similar to the constantinople or to use it ?

@MeastroZI We would want to avoid additional dependencies. Maybe it can act as a reference literature for our implementation? And yes, we should execute the expression only after we are sure it has no side effects.

@Vinit-Pandit
Copy link
Contributor

Vinit-Pandit commented Dec 3, 2024

I have researched a bit and play with the devtool and interested to go forward , i have one doubt that how can we find weather the function or method we have called from terminal is side effect free means , i have one way in my mind like this .

  • we will traverse from the AST nodes of the code

  • recursively traverse from the content of the function

  • have to maintain the object name context copy in which all the local identifiers nodes are mapped with there nodes ( AST node) so we can check the variable used under the function is global or local one . As if it working on local variable then it is not have any side effect .

  • this context copy can be nested because of closers property

  • now with this context copy we can safely define weather the function contain is side effect free or not .

@Snehil-Shah and @kgryte can u tell me am i on correct path or not

If not then i will greatly appreciate your guide

@Snehil-Shah
Copy link
Member

@MeastroZI Can you explain better with some examples?

Cases like mentioned in the OP and with global variables like:

[1]: var a = 'a string'

[2]: function modifyString(val) {
           // A pure operation
      }

[3]: modifyString(a)
      // Gives eager evaluated output 

@Vinit-Pandit
Copy link
Contributor

Vinit-Pandit commented Dec 3, 2024

@MeastroZI Can you explain better with some examples?

Cases like mentioned in the OP and with global variables like:

[1]: var a = 'a string'

[2]: function modifyString(val) {
           // A pure operation
      }

[3]: modifyString(a)
      // Gives eager evaluated output 

sure

  • this one is basic :
[1]: var a = 'a string'

[2]: function modifyString(val) {
           a = 'changes'
      }

[3]: modifyString(a)
      // nothing 
  • if we define local variable same name as a global one:

[1]: var a = 'a string'

[2]: function modifyString(val) {
          let a ; 
          a = 'another local string' ;
          return a ;
  
      }

[3]: modifyString(a)
// pure function
// another local string 

NOTE : in this case we require to maintain the local context as here we define the local variable and changing it

  • nested one
1]: var a = 'a string'

[2]: function modifyString(val) {
          let a ; 
           
           function modifyStringsecond() { 
                a = "changes from nested method";
            }
          modifyStringSecond() ;
          return a ;
  
      }

[3]: modifyString(a)
// pure function 
// change from nested method 

NOTE : in this case we need to context map need to be nested as there can be a variable define in execution context and have to check weather this variable is of local one or the global one ,

like in this case this same example is impure

1]: var a = 'a string'

[2]: function modifyString(val) {   
           function modifyStringsecond() { 
                a = "changes from nested method";
            }
          modifyStringSecond() ;
          return a ;
  
      }

[3]: modifyString(a)
// impure function 
// nothing

@Vinit-Pandit
Copy link
Contributor

Vinit-Pandit commented Dec 3, 2024

If i am in very wrong direction ! then please tell me , please don't just ignore me here. 😢

@Snehil-Shah
Copy link
Member

Snehil-Shah commented Dec 3, 2024

@MeastroZI you misunderstood my example

[1]: var a = "HELLO"

[2]: function modifyString(val) {
          return val.toLowerCase()
      }

[3]: modifyString(a)
      // Should eagerly evaluate to 'hello'

Plus also the examples in the RFC:

[1]: [ 1, 3, 2 ].sort()
      // Should eagerly evaluate

@Vinit-Pandit
Copy link
Contributor

@MeastroZI you misunderstood my example

[1]: var a = "HELLO"

[2]: function modifyString(val) {
          return val.toLowercase()
      }

[3]: modifyString(a)
      // Should eagerly evaluate to 'hello'

No i underdtood it and Completly agree with that to , but my question is how we can algorithmically we can define weather the method is side effect free or not ? Please refer this for completely understand my question. https://stackoverflow.com/questions/10662477/is-there-a-way-to-determine-if-a-javascript-function-has-side-effects#

@Vinit-Pandit
Copy link
Contributor

Specially if function is calling other funcrion and have nested call expression

@Snehil-Shah
Copy link
Member

Snehil-Shah commented Dec 3, 2024

@MeastroZI I don't know (haven't dived into this yet), I'll leave that for you to figure out.

@kgryte
Copy link
Member Author

kgryte commented Dec 3, 2024

As discussed during office hours today, the plan is to punt AST analysis for side effects to future work. Instead, at least for stdlib functions, the initial goal will be to add meta data to packages describing whether exposed APIs are pure. We can then build a REPL database of such functions, and for those functions, perform eager evaluation with result preview.

Before building such a database, the aim should be to implement the logic and UX for displaying result previews. As a test case, the implementation should assume that a single API (e.g., base.sin) is pure. Once we've figured out the UX, we can later extend support for result previews to other APIs.

kgryte added a commit that referenced this issue Feb 2, 2025
PR-URL: #4277
Ref: #2073
Co-authored-by: Athan Reines <[email protected]>
Co-authored-by: Snehil Shah <[email protected]>
Reviewed-by: Athan Reines <[email protected]>
Reviewed-by: Snehil Shah <[email protected]>
@kgryte
Copy link
Member Author

kgryte commented Feb 3, 2025

@Vinit-Pandit It looks like the eager evaluation logic isn't handling unary expressions. E.g.,

Image

Image

However, it does handle binary expressions

Image

@Vinit-Pandit
Copy link
Contributor

Vinit-Pandit commented Feb 3, 2025

Yeah, I just realized this yesterday. I’ll work on it. I also kind of miss the logic for eager evaluation of array and object literals 😅, like this:

[1, 3, 3] and { "ss": 6 }

Vinit-Pandit added a commit to Vinit-Pandit/stdlibMine that referenced this issue Feb 4, 2025
PR-URL: stdlib-js#4277
Ref: stdlib-js#2073
Co-authored-by: Athan Reines <[email protected]>
Co-authored-by: Snehil Shah <[email protected]>
Reviewed-by: Athan Reines <[email protected]>
Reviewed-by: Snehil Shah <[email protected]>
@Vinit-Pandit
Copy link
Contributor

Regarding the database for eager evaluation, in the current logic, I rely on the function call's code within the REPL. However, this has one drawback: if we store a reference to the function in another variable, eager evaluation does not occur—even if the API is pure.

Image

I believe we need some read-only metadata that always remains attached to the function API, something similar to this:

Image.

This way, whenever a function is called in the REPL, we can check its metadata before execution and ensure eager evaluation happens when applicable .

@Snehil-Shah
Copy link
Member

Snehil-Shah commented Feb 4, 2025

@Vinit-Pandit

if we store a reference to the function in another variable, eager evaluation does not occur—even if the API is pure.

I think that's fine.

The problem with attaching metadata directly to the function is, it can be hacked pretty easily, knowingly or unknowingly. What if the user defines their own impure function with the metadata set to isPure: true?

Plus its kinda weird mutating stdlib functions itself to support a REPL feature..

@kgryte
Copy link
Member Author

kgryte commented Feb 4, 2025

I agree with Snehil. If a user aliases a known alias, it is fine that we wouldn't perform eager evaluation. Plus, this has the added benefit that, if a user does not want to perform eager evaluation (e.g., due to a large ndarray, etc), then aliasing provides a temporary escape hatch.

kgryte pushed a commit that referenced this issue Feb 4, 2025
@kgryte kgryte reopened this Feb 4, 2025
@kgryte
Copy link
Member Author

kgryte commented Feb 4, 2025

BTW: @Vinit-Pandit and @Snehil-Shah Thanks for your efforts on this. Playing around with it in the REPL is pretty cool and definitely quite handy. I'm looking forward to rolling this out more widely once we get the database up and running.

saurabhraghuvanshii pushed a commit to saurabhraghuvanshii/stdlib-gs that referenced this issue Feb 11, 2025
PR-URL: stdlib-js#4277
Ref: stdlib-js#2073
Co-authored-by: Athan Reines <[email protected]>
Co-authored-by: Snehil Shah <[email protected]>
Reviewed-by: Athan Reines <[email protected]>
Reviewed-by: Snehil Shah <[email protected]>
saurabhraghuvanshii pushed a commit to saurabhraghuvanshii/stdlib-gs that referenced this issue Feb 11, 2025
ShabiShett07 pushed a commit to ShabiShett07/stdlib that referenced this issue Feb 26, 2025
PR-URL: stdlib-js#4277
Ref: stdlib-js#2073
Co-authored-by: Athan Reines <[email protected]>
Co-authored-by: Snehil Shah <[email protected]>
Reviewed-by: Athan Reines <[email protected]>
Reviewed-by: Snehil Shah <[email protected]>
ShabiShett07 pushed a commit to ShabiShett07/stdlib that referenced this issue Feb 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: 4 Likely to be moderately difficult. Enhancement Issue or pull request for enhancing existing functionality. JavaScript Issue involves or relates to JavaScript. Needs Discussion Needs further discussion. priority: Normal Normal priority concern or feature request. REPL Issue or pull request specific to the project REPL. RFC Request for comments. Feature requests and proposed changes.
Projects
None yet
Development

No branches or pull requests

3 participants