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

Allow environment variables and constants as default values #1607

Merged

Conversation

iamdefinitelyahuman
Copy link
Contributor

What I did

Allow builtin constants and environment variables to be used as defaults for keyword arguments - closes #1525

This does NOT allow custom constants.

How I did it

  • vyper/parser/expr.py - added BUILTIN_CONSTANTS and ENVIRONMENT_VARIABLES
  • vyper/signatures/function_signature.py - add checks to validate_default_values

How to verify it

Run the updated tests.

Description for the changelog

  • Allow constants and environment variables to be used as defaults for keyword arguments

Cute Animal Picture

image

@public
@payable
def foo(a: uint256(wei) = msg.value) -> bool:
self.xx = a
Copy link
Member

Choose a reason for hiding this comment

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

Do += here instead and call this twice in the test to show proper operation

@@ -190,7 +190,7 @@ def bar() -> uint256(wei):
c.foo(transact={'value': 31337})
assert c.bar() == 31337
c.foo(666, transact={'value': 9001})
assert c.bar() == 666
assert c.bar() == 31337 + 666
Copy link
Member

Choose a reason for hiding this comment

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

Maybe also add a line that says "balance of c contract is 31337 + 9001" to show the difference in using default args.

@@ -53,6 +53,23 @@
string_to_bytes,
)

# var name: (lllnode, type)
BUILTIN_CONSTANTS = {
'EMPTY_BYTES32': ([0], 'bytes32'),
Copy link
Member

Choose a reason for hiding this comment

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

style-wise I think the list constructor can be factored out here.

@charles-cooper
Copy link
Member

LGTM

@fubuloubu fubuloubu merged commit faacf15 into vyperlang:master Sep 10, 2019
@iamdefinitelyahuman iamdefinitelyahuman deleted the expanded-default-kwargs branch September 12, 2019 13:44
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: Allow environment variables and constants as "default value" in public functions
3 participants