-
-
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[tool]: improvements to AST annotation #3829
feat[tool]: improvements to AST annotation #3829
Conversation
cbe8d5e
to
a4ef57e
Compare
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #3829 +/- ##
===========================================
- Coverage 86.15% 69.69% -16.46%
===========================================
Files 92 92
Lines 13921 13990 +69
Branches 3067 3075 +8
===========================================
- Hits 11993 9751 -2242
- Misses 1491 3570 +2079
- Partials 437 669 +232 ☔ View full report in Codecov by Sentry. |
fix module_node for Module
add typeclass to all types add VyperType.to_dict to handle type serialization in AST improve VyperType.decl_node handling for all types add _addl_node_fields to handle types which "need parsing", e.g. HashMap, bytestrings, dynamic and static arrays, tuples
remove type information as it clutters the variable analysis test
it's only used in a couple tests, and those can be rewritten
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.
Shouldn't AdressT
, SelfT
, HashMapT
have typeclass
assigned?
i found it not very useful (especially for AddressT and SelfT) because the typeclass is the same as the type name. but maybe yea we should add it for hashmap |
clear demarcation between _id (might be an actual identifier) and typeclass ("class" type for 3rd party tooling)
What I did
some improvements to AST annotation, including
i also removed some dead code as i came across it.
compare_nodes
is only used in a couple tests, and those can be rewritten with the__eq__()
operator. i'm not very thrilled about the idea of node equality in general as it's a bit ill-defined (what exactly do we want to measure?) and i think we should try to move away from defining/relying on node equality at all.along with #3790, should fix #3600
How I did it
How to verify it
Commit message
Description for the changelog
Cute Animal Picture