fix: iterator modification analysis

[charles-cooper]

What I did

fix #3429

How I did it

How to verify it

Commit message

this commit fixes several bugs with analysis of iterator modification in
loops. to do so, it refactors the analysis code to track reads/writes
more accurately, and uses analysis machinery instead of AST queries to
perform the check. it enriches ExprInfo with an `attr` attribute, so
this can be used to detect if an ExprInfo is derived from an
`Attribute`.

ExprInfo could be further enriched with `Subscript` info
so that the Attribute/Subscript chain can be reliably recovered just
from ExprInfos, especially in the future if other functions rely on
being able to recover the attribute chain.

this commit also modifies `validate_functions` so that it validates the
functions in dependency (call graph traversal) order rather than the
order they appear in the AST.

refactors:
- add `enter_for_loop()` context manager for convenience+clarity
- remove `ExprInfo.attribute_chain`, it was too confusing
- hide `ContractFunctionT` member variables (`_variable_reads`,
  `_variable_writes`, `_used_modules`) behind public-facing API
- remove `get_root_varinfo()` in favor of a helper
  `_get_variable_access()` function which detects access on variable
  sub-members (e.g., structs).

Description for the changelog

Cute Animal Picture

[codecov-commenter]

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (37ef8f4) 84.94% compared to head (591b97b) 84.71%.
Report is 2 commits behind head on master.

Files	Patch %	Lines
vyper/semantics/analysis/local.py	97.52%	1 Missing and 2 partials ⚠️
vyper/codegen/expr.py	92.85%	0 Missing and 1 partial ⚠️

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

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3764      +/-   ##
==========================================
- Coverage   84.94%   84.71%   -0.24%     
==========================================
  Files          92       92              
  Lines       13594    13660      +66     
  Branches     3060     3067       +7     
==========================================
+ Hits        11548    11572      +24     
- Misses       1560     1596      +36     
- Partials      486      492       +6     

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

[tserg]

It is a nit, but I think we should use analyze instead of analyse in the names since the former is predominantly used.

Also, this contract fails to compile when the iterator is a struct member that is not modified by the loop, but another struct member is. Is this intended behaviour?

struct Foo:
    a: uint256[2]
    b: uint256

f: Foo

@deploy
def __init__():
    self.f = Foo({a: [256, 512], b: 321})

@external
def bar():
    for i: uint256 in self.f.a:
        self._bar()

@internal
def _bar():
    self.f.b = 123

vyper.exceptions.ImmutableViolation: Cannot modify loop variable `f`

  contract "tmp/modify_struct_iter.vy:5", line 5:0 
       4
  ---> 5 f: Foo
  -------^
       6

  contract "tmp/modify_struct_iter.vy:14", function "bar", line 14:8 
       13     for i: uint256 in self.f.a:
  ---> 14         self._bar()
  ----------------^
       15

[charles-cooper]

Also, this contract fails to compile when the iterator is a struct member that is not modified by the loop, but another struct member is. Is this intended behaviour?

hm this seems like a false positive. i am less worried about false positives than false negatives, but i will look into it!

[charles-cooper]

i have a draft fix for the struct member issue, although it doesn't feel very clean yet: charles-cooper#22