-
-
Notifications
You must be signed in to change notification settings - Fork 804
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: allow functions to be named after builtins and reserved keywords #3307
feat: allow functions to be named after builtins and reserved keywords #3307
Conversation
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## master #3307 +/- ##
=======================================
Coverage 88.95% 88.96%
=======================================
Files 84 84
Lines 10606 10605 -1
Branches 2214 2213 -1
=======================================
Hits 9435 9435
+ Misses 767 766 -1
Partials 404 404
... and 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
vyper/semantics/namespace.py
Outdated
namespace = get_namespace() | ||
if attr in namespace and attr not in [x for i in namespace._scopes for x in i]: | ||
raise NamespaceCollision(f"Cannot assign to '{attr}', it is a builtin") | ||
if attr.lower() in RESERVED_KEYWORDS or attr.upper() in OPCODES: | ||
raise StructureException(f"'{attr}' is a reserved keyword") | ||
|
||
|
||
def validate_identifier_name(attr): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can stay as validate_identifier
vyper/semantics/namespace.py
Outdated
@@ -119,16 +120,23 @@ def override_global_namespace(ns): | |||
_namespace = tmp | |||
|
|||
|
|||
def validate_identifier(attr): | |||
def validate_namespace_availability(attr): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we change the arg name to ident
?
vyper/semantics/namespace.py
Outdated
if not re.match("^[_a-zA-Z][a-zA-Z0-9_]*$", attr): | ||
raise StructureException(f"'{attr}' contains invalid character(s)") | ||
|
||
|
||
# Reserved python keywords | ||
PYTHON_KEYWORDS = set({"if", "for", "while", "pass", "def", "assert", "continue", "raise"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this also does not need the set constructor!
@@ -421,22 +420,6 @@ def foo(): | |||
), | |||
( | |||
""" | |||
struct X: | |||
bar: int128 | |||
decimal: int128 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i guess these are allowed now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes they are, similar to def decimal(): pass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so this is looking pretty good to me. my only question is if we should hold off until the 0.4.x series to merge, since it is kind of a breaking change (although in the sense that the compiler will accept more programs than before) to the language.
discussed with @fubuloubu offline, merging since it should not break existing programs |
What I did
Since functions are in the
self
namespace, we can support function names that are identical to builtins and reserved keywords. This applies to both internal and external functions.Also fixes #3227, and supercedes #3244.
How I did it
Split
validate_identifier
intovalidate_identifier_with_namespace
(which is skipped with theskip_namespace_validation
flag) andvalidate_identifier_name
(which should always be run).How to verify it
See test.
Commit message
Description for the changelog
Allow functions to be named after builtins and reserved keywords
Cute Animal Picture