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

refactor: reimplement AST folding #3669

Merged
merged 189 commits into from
Dec 31, 2023

Conversation

tserg
Copy link
Collaborator

@tserg tserg commented Nov 10, 2023

What I did

fixes #3363, #3344

How I did it

How to verify it

Commit message

refactor: reimplement AST folding

this commit reimplements AST folding. fundamentally, it changes AST
folding from a mutating pass to be an annotation pass. this brings
several benefits:
- typechecking is easier, because folding does not have to reason at all
  about types. type checking happens on both the folded and unfolded
  nodes, so intermediate values are type-checked.
- correctness in general is easier, because the AST is not mutated.
  there is also some incidental performance benefit, although that is
  not necessarily the focus here.
- the vyper frontend is now nearly mutation-free. only the getter AST
  expansion pass remains.

note that we cannot push folding past the typechecking stage entirely,
because some type checking operations depend on having folded values
(e.g., `range()` expressions, or type expressions with integer
parameters).

the approach taken in this commit is to change constant folding to be
annotating, rather than mutating. this way, type-checking can operate on
the original AST (and check for the folded values where needed).
intermediate values are also type-checked, so expressions like
`x: uint128 = 2**128 + 1 - 1` are caught by the typechecker.

summary of changes:
- `evaluate()` is renamed to `_try_fold()`. a new utility function
  called `get_folded_value()` caches folded values and is threaded
  through the codebase.
- `pre_typecheck` is added, which extracts `constant` variables and runs
  `get_folded_value()` on all nodes.
- a new `Modifiability` enum replaces the old (confusing) `is_constant`
  and `is_immutable` attributes on ExprInfo.
- `ExprInfo.is_transient` is removed, and handled by adding `TRANSIENT`
  to the `DataLocation` enum.
- the old `check_literal` and `check_kwargable` utility functions are
  replaced with a more general (and more correct) `check_modifiability`
  function
- several utility functions (ex. `_validate_numeric_bounds()`) related
  to ad-hoc type-checking (which would happen during constant folding)
  are removed.
- `CompilerData.vyper_module_folded` is renamed to
  `annotated_vyper_module`
- the AST output options are now `ast` and `annotated_ast`.
- `None` literals are now banned in AST validation instead of during
  analysis.

Description for the changelog

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

vyper/builtins/_signatures.py Fixed Show fixed Hide fixed
vyper/semantics/types/function.py Fixed Show fixed Hide fixed
vyper/ast/nodes.py Fixed Show fixed Hide fixed
vyper/ast/nodes.py Fixed Show fixed Hide fixed
vyper/ast/nodes.py Fixed Show fixed Hide fixed
vyper/ast/nodes.py Fixed Show fixed Hide fixed
vyper/semantics/analysis/pre_typecheck.py Fixed Show fixed Hide fixed
Comment on lines +116 to +122
"""
y: constant(String[5]) = "szabo"
x: constant(uint256) = as_wei_value(5, y)

@external
def foo():
a: uint256 = x
Copy link
Member

Choose a reason for hiding this comment

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

some of these tests should really go in tests/functional/builtins/folding/

vyper/codegen/expr.py Fixed Show fixed Hide fixed
FOO: constant(uint256) = 3
BAR: constant(uint256) = 5
BAZ: constant(uint256) = 19
BAX: constant(uint256) = uint256_addmod(FOO, BAR, BAZ)
Copy link
Member

Choose a reason for hiding this comment

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

i think typically in computer science lore the next name in the list is QUX :D

Copy link
Member

Choose a reason for hiding this comment

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

but i like BAX too tbh

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i think typically in computer science lore the next name in the list is QUX :D

Oops I was not aware of this!

Copy link
Member

@charles-cooper charles-cooper left a comment

Choose a reason for hiding this comment

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

nice work -- vyper is looking much cleaner now!

@charles-cooper charles-cooper changed the title feat: add pre-folding refactor: reimplement AST folding Dec 31, 2023
@charles-cooper
Copy link
Member

@cyberthirst please take a look and see if you think there are any regressions in this PR as it's a bit large!

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.

constant folding of some builtins results in compiler panic
3 participants