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

feat: add epsilon builtin #3057

Merged
merged 6 commits into from
Aug 9, 2022
Merged

feat: add epsilon builtin #3057

merged 6 commits into from
Aug 9, 2022

Conversation

z80dev
Copy link
Contributor

@z80dev z80dev commented Aug 8, 2022

What I did

Added an epsilon() builtin which returns the smallest non-zero value for a decimal type.

Closes #2992

How I did it

In vyper/builtin_functions/functions.py I implemented a new class Epsilon. The function that this class represents should always be folded.

Instead of adding to the duplication between MinMaxValue and MethodID, I created a new base class FoldedFunction which contains all the logic for enforcing a function should always be folded.

In Epsilon's evaluate function, we check the type that was passed in, ensure it is a FixedAbstractType (which is the parent class for Decimal and any other decimal classes to be supported later).

Then we call the appropriate _eval_<type> function after checking which type was passed in. This currently only supports "decimal" but sets up a pattern for adding support for other decimal types in the future.

How to verify it

Run the new test, which checks a compiled contract's return value against a folded node.

Commit message

Implement epsilon for decimal types

Description for the changelog

Cute Animal Picture

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

# this check seems redundant, but sets a pattern to be followed
# when new decimal types are created
if isinstance(input_type, DecimalDefinition):
val = self._eval_decimal(input_type)
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 you can just inline _eval_decimal here. the use of that pattern elsewhere is to deal with some kind of inheritance, not needed here.

@@ -120,6 +121,22 @@
SHA256_BASE_GAS = 60
SHA256_PER_WORD_GAS = 12

class FoldedFunction(BuiltinFunction):
# Base class for nodes which should always be folded
_id = ""
Copy link
Member

Choose a reason for hiding this comment

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

probably can leave this out -- that way if the subclass does not supply _id, accessing the attribute will fail explicitly rather than silently

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point!

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.

very nice work! left a couple comments

@codecov-commenter
Copy link

codecov-commenter commented Aug 9, 2022

Codecov Report

Merging #3057 (1a3424a) into master (22ae74c) will decrease coverage by 1.45%.
The diff coverage is 73.33%.

❗ Current head 1a3424a differs from pull request most recent head 45d27a2. Consider uploading reports for the commit 45d27a2 to get more accurate results

@@            Coverage Diff             @@
##           master    #3057      +/-   ##
==========================================
- Coverage   88.34%   86.88%   -1.46%     
==========================================
  Files          97       97              
  Lines       10963    10965       +2     
  Branches     2590     2528      -62     
==========================================
- Hits         9685     9527     -158     
- Misses        830      964     +134     
- Partials      448      474      +26     
Impacted Files Coverage Δ
vyper/builtin_functions/functions.py 86.58% <73.33%> (-3.71%) ⬇️
vyper/builtin_functions/convert.py 67.38% <0.00%> (-23.66%) ⬇️
vyper/codegen/arithmetic.py 67.09% <0.00%> (-17.52%) ⬇️
vyper/semantics/types/value/numeric.py 80.45% <0.00%> (-4.60%) ⬇️
vyper/utils.py 80.86% <0.00%> (-3.83%) ⬇️
vyper/ir/s_expressions.py 91.17% <0.00%> (-2.95%) ⬇️
vyper/ast/nodes.py 91.68% <0.00%> (-1.50%) ⬇️
vyper/ir/optimizer.py 97.56% <0.00%> (-0.31%) ⬇️
vyper/codegen/abi_encoder.py 88.46% <0.00%> (-0.11%) ⬇️
... and 2 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@charles-cooper charles-cooper enabled auto-merge (squash) August 9, 2022 14:45
@charles-cooper charles-cooper changed the title Epsilon feat: add epsilon builtin Aug 9, 2022
@charles-cooper charles-cooper merged commit 05406af into vyperlang:master Aug 9, 2022
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.

VIP: epsilon(decimal) builtin for decimal types
3 participants