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

First version of partial evaluator: support for +,-,* operators and associative rules #528

Merged
merged 25 commits into from
Jul 15, 2024

Conversation

jeshecdom
Copy link
Contributor

@jeshecdom jeshecdom commented Jul 4, 2024

Towards #435.

  • First version of partial evaluator. Currently it is able to simplify expressions involving +, -, and *. However, there are some algebraic rules involving those operators that have not been added yet.

  • I decided to keep the partial evaluator separate from the constant evaluator because the partial evaluator still does not support certain language constructions, like structs: the partial evaluator needs to create AST nodes out of values, and there is currently no code to construct ASTStruct nodes out of StructValue. This will be added in the future.

  • Currently, to call the partial evaluator, one needs to call the function partiallyEvalExpression. In the future, the constant evaluator will call the partial evaluator, because they should give the same result for expressions that can fully evaluate.

  • Many algebraic rules are still pending, including the rest of integer operators and boolean operators.

  • Still lacks documentation, specially the explanation of how each rule works.

  • Since the partial evaluator is currently isolated from the rest of the code, the tests directly check the partial evaluator instead of going through a contract.

  • I have updated CHANGELOG.md

  • [ ] I have documented my contribution in Tact Docs: https://github.com/tact-lang/tact-docs/pull/PR-NUMBER

  • I have added tests to demonstrate the contribution is correctly implemented: this usually includes both positive and negative tests, showing the happy path(s) and featuring intentionally broken cases

  • I have run all the tests locally and no test failure was reported

  • I have run the linter, formatter and spellchecker

  • I did not do unrelated and/or undiscussed refactorings

There are still some TODOs (so, this is only a partial commit):
- Implementation of association rule 3 and simple algebraic rules (to be added to file algebraic.ts)
described in comments of issue: #435
- Processing of structs, function calls, id lookups, ternary conditional operator.
- Extensive documentation, specially in the description of what each rule does.
- Extensive testing of the rewrite rules.
…Priorities are no longer a property of rules, but are managed by the optimizer.
…ded a function to call the partial evaluator in the interpreter to ease testing.
@anton-trunov
Copy link
Member

@jeshecdom awesome stuff! could you please update your PR and pull in the most recent changes from main and also resolve the merge conflicts?

@anton-trunov anton-trunov self-assigned this Jul 5, 2024
Copy link
Member

@anton-trunov anton-trunov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point I did some superficial code style review mostly, let's merge things, run the linter with the newly introduced checks and then I'll switch to reviewing the algorithm. (from a quick glance it's impressive already)

src/constEval.ts Outdated Show resolved Hide resolved
src/constEval.ts Outdated Show resolved Hide resolved
src/optimizer/types.ts Outdated Show resolved Hide resolved
src/optimizer/types.ts Outdated Show resolved Hide resolved
src/optimizer/types.ts Outdated Show resolved Hide resolved
src/optimizer/util.ts Outdated Show resolved Hide resolved
src/optimizer/util.ts Outdated Show resolved Hide resolved
src/optimizer/util.ts Outdated Show resolved Hide resolved
src/test/e2e-emulated/partial-eval.spec.ts Outdated Show resolved Hide resolved
…-lang/tact/pull/528/files#r1666596822)

- Added `dummySrcInfo` as default parameter values to functions `__evalUnaryOp` and `__evalBinaryOp`. This way I was able to eliminate them and simply leave `evalUnaryOp` and `evalBinaryOp` as exportable functions. (https://github.com/tact-lang/tact/pull/528/files#r1666598821)
- Renamed ValueExpression for AstValue (https://github.com/tact-lang/tact/pull/528/files#r1666602072)
- Used Dummy interval in grammar.ts (https://github.com/tact-lang/tact/pull/528/files#r1666602072)
- Changed interfaces for abstract classes (https://github.com/tact-lang/tact/pull/528/files#r1666604615)
- Renamed `areEqualExpressions` to `eqExpressions`, and moved it to `ast.ts`. Used `eqNames` and moved `ast1.kind === ast2.kind` check before switch. (https://github.com/tact-lang/tact/pull/528/files#r1666609433)
@anton-trunov
Copy link
Member

There are merge conflicts

src/grammar/ast.ts Show resolved Hide resolved
src/optimizer/types.ts Outdated Show resolved Hide resolved
src/optimizer/util.ts Outdated Show resolved Hide resolved
src/constEval.ts Outdated Show resolved Hide resolved
src/constEval.ts Show resolved Hide resolved
- Added test cases for eqExpressions function in ast.ts.
- Changed if statement for conditional expressions in optimizer/util.ts.
- Changed lookupID for lookupName and evalX for fullyEvalX in constEval.ts.
src/grammar/ast.ts Outdated Show resolved Hide resolved
src/grammar/ast.ts Outdated Show resolved Hide resolved
src/optimizer/util.ts Outdated Show resolved Hide resolved
src/test/e2e-emulated/expr-equality.spec.ts Outdated Show resolved Hide resolved
- Moved AstValue to ast.ts.
- Removed try/catch from eqExpressions in ast.ts.
- Removed `traverse` from ast.ts since it has been already moved to iterators.ts
- Moved expr-equality.spec.ts and partial-eval.spec.ts to src/grammar/test
- Added test cases for `isValue' function in ast.ts in src/grammar/test/expr-is-value.spec.ts.
@anton-trunov anton-trunov merged commit 1df1943 into main Jul 15, 2024
3 checks passed
@anton-trunov anton-trunov deleted the partial_evaluator branch July 15, 2024 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants