Skip to content
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

Introduce EvaluationContext for statement and expression evaluation #979

Merged
merged 2 commits into from Oct 22, 2021

Conversation

lahma
Copy link
Collaborator

@lahma lahma commented Oct 19, 2021

Needed in generators and also other benefits, took out of generators as it's getting big enough PR without this.

  • state information can flow from engine entry inside single context, internal EvaluationContext
  • now _engine * field is no longer required in statement or expression base classes (less memory usage)
  • should also open scenarios like per call constraints instead of global ones fixed in engine, like
    • engine.Evaluate("my script", perCallOptions => DoDonfigure(perCallOptions))
  • this should also separate better evaluation time information and engine infrastructure, maybe at some point Engine doesn't need to be passed inside the context at all

Biggest API change is that I've obsoleted JsValue.Invoke and now requiring engine parameter. It's a bit scary invoke facility outside of engine anyway.

@lahma lahma force-pushed the statement-eval-context-param branch from 72d40e3 to 491fa17 Compare October 22, 2021 07:34
@lahma
Copy link
Collaborator Author

lahma commented Oct 22, 2021

Also extracted Completion handling from the old generators PR.

Now expression evaluation produces ExpressionResult which hold object result, which also allows Reference, old logic had object return type all around which can cause boxing of Completion, now statements return Completion and there's conversion to between ExpressionResult and Completion. Statements never return references so Completion's JsValue is always valid.

I was able to squash couple of bugs also related to array pattern handling and simplified DeclarativeEnvironmentRecord a bit

@sebastienros
Copy link
Owner

Can you summarize the breaking changes in the API ? Apart from the Invoke change.

@lahma lahma merged commit bf140bf into sebastienros:main Oct 22, 2021
@lahma lahma deleted the statement-eval-context-param branch October 22, 2021 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants