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

refactor: simplify GlobalContext #3209

Merged
merged 17 commits into from
Jan 5, 2023

Conversation

charles-cooper
Copy link
Member

@charles-cooper charles-cooper commented Jan 1, 2023

What I did

Substantially simplify GlobalContext. Most of the code was dead, and required either just deleting or in some cases, also updating usages with "new-style" usages. Also delete vyper/ast/signatures/interface.py, since that was only used in GlobalContext.

also renames ContractFunction to ContractFunctionT for consistency with other types.

will merge FunctionSignature and ContractFunctionT in a follow-up PR.

How I did it

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

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

@codecov-commenter
Copy link

codecov-commenter commented Jan 1, 2023

Codecov Report

Merging #3209 (6a012be) into master (db88b1a) will increase coverage by 0.50%.
The diff coverage is 91.97%.

❗ Current head 6a012be differs from pull request most recent head 763144b. Consider uploading reports for the commit 763144b to get more accurate results

@@            Coverage Diff             @@
##           master    #3209      +/-   ##
==========================================
+ Coverage   88.43%   88.93%   +0.50%     
==========================================
  Files          88       84       -4     
  Lines       11047    10581     -466     
  Branches     2338     2206     -132     
==========================================
- Hits         9769     9410     -359     
+ Misses        823      760      -63     
+ Partials      455      411      -44     
Impacted Files Coverage Δ
vyper/codegen/return_.py 94.73% <ø> (-0.14%) ⬇️
vyper/semantics/analysis/utils.py 92.56% <ø> (ø)
vyper/utils.py 83.08% <ø> (-0.73%) ⬇️
vyper/codegen/keccak256_helper.py 82.60% <60.00%> (+0.79%) ⬆️
vyper/ast/signatures/function_signature.py 88.28% <66.66%> (-1.27%) ⬇️
vyper/codegen/events.py 93.33% <75.00%> (ø)
vyper/codegen/ir_node.py 91.03% <75.00%> (-0.69%) ⬇️
vyper/semantics/types/bytestrings.py 85.54% <83.33%> (+2.61%) ⬆️
vyper/codegen/abi_encoder.py 88.88% <85.71%> (+0.31%) ⬆️
vyper/codegen/expr.py 88.19% <86.27%> (+3.59%) ⬆️
... and 31 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@charles-cooper charles-cooper changed the title [wip] refactor: merge FunctionSignature and ContractFunction [wip] refactor: simplify GlobalContext Jan 1, 2023
@charles-cooper charles-cooper changed the title [wip] refactor: simplify GlobalContext refactor: simplify GlobalContext Jan 1, 2023
@charles-cooper charles-cooper marked this pull request as ready for review January 1, 2023 18:13
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

@@ -4,7 +4,7 @@
from vyper.codegen.stmt import parse_body
from vyper.semantics.analysis.local import FunctionNodeVisitor
from vyper.semantics.namespace import Namespace, override_global_namespace
from vyper.semantics.types.function import ContractFunction, FunctionVisibility, StateMutability
from vyper.semantics.types.function import ContractFunctionT, FunctionVisibility, StateMutability

Check notice

Code scanning / CodeQL

Cyclic import

Import of module [vyper.semantics.types.function](1) begins an import cycle.
@@ -6,7 +6,7 @@
get_possible_types_from_node,
)
from vyper.semantics.types import TYPE_T, EnumT, EventT, SArrayT, StructT, is_type_t
from vyper.semantics.types.function import ContractFunction, MemberFunctionT
from vyper.semantics.types.function import ContractFunctionT, MemberFunctionT

Check notice

Code scanning / CodeQL

Cyclic import

Import of module [vyper.semantics.types.function](1) begins an import cycle.
@@ -43,7 +43,7 @@
TupleT,
is_type_t,
)
from vyper.semantics.types.function import ContractFunction, MemberFunctionT, StateMutability
from vyper.semantics.types.function import ContractFunctionT, MemberFunctionT, StateMutability

Check notice

Code scanning / CodeQL

Cyclic import

Import of module [vyper.semantics.types.function](1) begins an import cycle.
@@ -19,7 +19,7 @@
from vyper.semantics.analysis.utils import validate_expected_type, validate_unique_method_ids
from vyper.semantics.namespace import get_namespace
from vyper.semantics.types.base import VyperType
from vyper.semantics.types.function import ContractFunction
from vyper.semantics.types.function import ContractFunctionT

Check notice

Code scanning / CodeQL

Cyclic import

Import of module [vyper.semantics.types.function](1) begins an import cycle.
Copy link
Member

@fubuloubu fubuloubu left a comment

Choose a reason for hiding this comment

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

Looks pretty good, minus the cyclic import warning from CodeQL

@charles-cooper
Copy link
Member Author

those warnings were already there, just got retriggered because those lines changed

@charles-cooper charles-cooper merged commit a16af3d into vyperlang:master Jan 5, 2023
@charles-cooper charles-cooper deleted the refactor/signatures branch January 5, 2023 04:43
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.

3 participants