-
-
Notifications
You must be signed in to change notification settings - Fork 807
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: guard against mutating code in non-mutable functions #3555
Conversation
@@ -76,16 +76,111 @@ def foo(): | |||
for i in range(x): | |||
pass""", | |||
""" | |||
f: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.
This test now throws for its other issue of call violation, which is checked first in visit_Expr
. Hence, I removed it as there are already tests for call violation in this file.
get_possible_types_from_node, | ||
) | ||
from vyper.semantics.types import TYPE_T, BoolT, EnumT, EventT, SArrayT, StructT, is_type_t | ||
from vyper.semantics.types.function import ContractFunctionT, MemberFunctionT | ||
from vyper.semantics.types.function import ContractFunctionT, MemberFunctionT, StateMutability |
Check notice
Code scanning / CodeQL
Cyclic import
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #3555 +/- ##
==========================================
- Coverage 89.14% 89.13% -0.02%
==========================================
Files 85 85
Lines 11362 11378 +16
Branches 2584 2587 +3
==========================================
+ Hits 10129 10142 +13
- Misses 812 815 +3
Partials 421 421
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
mutable_builtins = ( | ||
"create_minimal_proxy_to", | ||
"create_copy_of", | ||
"create_from_blueprint", | ||
) |
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.
There is an overlap here with #3546 (comment).
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.
Another option here could be to check constancy of the context in _CreateBase.build_IR()
, similar to how it is being checked for raw_call
. However, this would not resolve #3546.
@@ -6,6 +6,7 @@ | |||
from vyper.codegen.expr import Expr | |||
from vyper.codegen.ir_node import IRnode | |||
from vyper.exceptions import CompilerPanic, TypeMismatch | |||
from vyper.semantics.analysis.base import StateMutability |
Check notice
Code scanning / CodeQL
Cyclic import
@@ -51,7 +51,7 @@ | |||
UnfoldableNode, | |||
ZeroDivisionException, | |||
) | |||
from vyper.semantics.analysis.base import VarInfo | |||
from vyper.semantics.analysis.base import StateMutability, VarInfo |
Check notice
Code scanning / CodeQL
Cyclic import
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.
viwed
superseded by #3729 |
What I did
Fix #3554.
How I did it
Move the checks from
visit_Expr
tovisit_Call
. This causes semantic analysis to leak intoannotation.py
, but it should be fixed later on by #3456 when annotation and validation are combined into a single pass.How to verify it
See tests.
Commit message
Description for the changelog
Guard against mutating code in non-mutable functions
Cute Animal Picture