-
-
Notifications
You must be signed in to change notification settings - Fork 803
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
fix[ux]: internal functions not in metadata #4328
base: master
Are you sure you want to change the base?
fix[ux]: internal functions not in metadata #4328
Conversation
vyper/compiler/output.py
Outdated
|
||
for fn_t in module_t.exposed_functions: | ||
assert isinstance(fn_t.ast_def, vy_ast.FunctionDef) | ||
for inf_t in fn_t.reachable_internal_functions: |
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.
what does inf_t
stand for?
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.
internal function and the t cause its the type - ill rename it to something clearer
assert "foochino" in out["function_info"] | ||
assert "bar" in out["function_info"] | ||
assert "foo" in out["function_info"] | ||
assert "faa" not in out["function_info"] |
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.
hmm noticing something messed up -- what if we have two functions with the same name in different modules?
vyper/compiler/output.py
Outdated
for fn_t in module_t.exposed_functions: | ||
assert isinstance(fn_t.ast_def, vy_ast.FunctionDef) | ||
for rif_t in fn_t.reachable_internal_functions: | ||
sigs[rif_t.ast_def.module_node.path + ": " + rif_t.name] = rif_t |
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 think there can actually be collisions between path
since it's not resolved (e.g. depending on which search path is used)
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 think we need to find a way to export the internal functions which doesn't have any collisions in the dict. i.e. it is injective from reachable functions to functions which actually appear in the output.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4328 +/- ##
==========================================
- Coverage 90.99% 88.74% -2.25%
==========================================
Files 112 112
Lines 16017 16032 +15
Branches 2696 2698 +2
==========================================
- Hits 14574 14227 -347
- Misses 1005 1289 +284
- Partials 438 516 +78 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
changed it to use |
sorry, |
What I did
Add internal functions from imported modules into metadata output. Issue #4312 but reverse the imports.
How I did it
Print all reachable functions rather than just functions that are present in the module.
How to verify it
Commit message
Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)
Description for the changelog
Cute Animal Picture