AVM: Simplify conversion and fix a spurious complaint from static analysis #5421
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Static analysis tools noted that:
```
for i, sv := range cx.stack {
- stack[i] = stackValueToTealValue(&sv)
```
was a potential problem because `sv` is always the same variable in a
for loop, so this code was passing the same address on each call.
That was fine, since stackValueToTealValue is single-threaded, and
does not sture the address. It's just a simple conversion.
But, noting that the conversion was more complicated than it needed to
be, and probably allocated more than it needed to, this PR simplifies
it and makes it work on stackValue values instead of pointers.
jannotti
force-pushed
the
simplify-debugger-conversion
branch
from
b0a1ab0
to
a36c35f
Compare
bbroder-algo
approved these changes
May 26, 2023
Codecov Report
@@ Coverage Diff @@
## master #5421 +/- ##
==========================================
+ Coverage 55.32% 55.33% +0.01%
==========================================
Files 452 452
Lines 63851 63844 -7
==========================================
+ Hits 35328 35331 +3
+ Misses 26094 26086 -8
+ Partials 2429 2427 -2
... and 5 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
ahangsu
approved these changes
May 26, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Static analysis tools noted that:
was a potential problem because
svis always the same variable in a for loop, so this code was passing the same address on each call. That was fine, since stackValueToTealValue is single-threaded, and does not store the address. It's just a simple conversion.But, noting that the conversion was more complicated than it needed to be, and probably allocated more than it needed to, this PR simplifies it and makes it work on stackValue values instead of pointers.
Also removes the unused function: valueDeltaToValueDelta and associated test.
Test Plan
Existing tests