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

fix[lang]: pure access analysis #3895

Merged
merged 9 commits into from
Apr 2, 2024

Conversation

charles-cooper
Copy link
Member

@charles-cooper charles-cooper commented Mar 26, 2024

What I did

fix #3894, fix #3830

use analysis instead of pattern matching on syntax.
update code style for the pure decorator tests

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-->

fix access to module variables in pure functions. the
`_validate_self_reference()` utility function was hard-coded to check
the "self" name; remove it and replace with an analysis-based check.

misc/refactor:
- rename `VarInfo.is_module_variable()` to more fitting
  `VarInfo.is_state_variable()`.
- module storage variable
- immutable variable
- module immutable variable
@@ -38,7 +38,7 @@
from vyper.semantics.data_locations import DataLocation

# TODO consolidate some of these imports
from vyper.semantics.environment import CONSTANT_ENVIRONMENT_VARS, MUTABLE_ENVIRONMENT_VARS
from vyper.semantics.environment import CONSTANT_ENVIRONMENT_VARS

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
vyper.semantics.environment
begins an import cycle.
@codecov-commenter
Copy link

codecov-commenter commented Mar 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.07%. Comparing base (4595938) to head (79d5edf).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3895      +/-   ##
==========================================
- Coverage   86.34%   86.07%   -0.28%     
==========================================
  Files          92       92              
  Lines       14028    14036       +8     
  Branches     3083     3085       +2     
==========================================
- Hits        12113    12081      -32     
- Misses       1487     1517      +30     
- Partials      428      438      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fubuloubu
Copy link
Member

Is this a bug or an optimization?

@cyberthirst
Copy link
Collaborator

this isn't right?

src = """
first: public(uint256)
second: public(uint256)

@internal
@pure
def foo() -> uint256:
    return self.codesize

@deploy
def __init__():
    self.first = self.foo()


@external
def bar():
    self.second = self.foo() 
"""

c = b.loads(src)
print(c.first())
c.bar()
print(c.second())

outputs:

201
136

@charles-cooper
Copy link
Member Author

this isn't right?

src = """
first: public(uint256)
second: public(uint256)

@internal
@pure
def foo() -> uint256:
    return self.codesize

@deploy
def __init__():
    self.first = self.foo()


@external
def bar():
    self.second = self.foo() 
"""

c = b.loads(src)
print(c.first())
c.bar()
print(c.second())

outputs:

201
136

hm yea we should probably ban codesize (it's not currently classified as a state variable)

@charles-cooper charles-cooper merged commit 20432c5 into vyperlang:master Apr 2, 2024
147 checks passed
@charles-cooper charles-cooper deleted the fix/pure-access branch April 2, 2024 13:55
@pcaversaccio
Copy link
Collaborator

pcaversaccio commented Apr 2, 2024

This PR introduced a regression. Tracking here: #3902.

electriclilies pushed a commit to electriclilies/vyper that referenced this pull request Apr 27, 2024
this commit fixes the ability to access module variables in pure
functions. the `_validate_self_reference()` utility function was
hard-coded to check the "self" name; remove it and replace with an
analysis-based check.

this commit also fixes the pure access check for immutable variables,
and address members (e.g. `.codesize`)

misc/refactor:
* rename `VarInfo.is_module_variable()` to more fitting
  `VarInfo.is_state_variable()`.
* refactor pure decorator tests to be in line with
  recent best practices
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.

immutables accessible in pure context accessing state in pure function
6 participants