algorand / algorand/pyteal

Source Mapper Improvements: Small Fixes

Open
#681 0 comments 0 reactions 0 assignees View on GitHub
new-feature-request Team Scytale
Dominant language
Python
Stars
288
Forks
138
PR merge metrics
No merged PRs in 30d

Description

## Problem

There were a few issues that were not resolved at the time of merging #650 .

## Solution - Implement the following small improvements

- [x] (#650 already addressed) Clarify the legend for `_PyTealMapper.annotate()` (cf: https://github.com/tzaffi/pyteal/blob/f2a945a8eeb396e9c544b62cd3d1a6383bfc9bff/pyteal/compiler/sourcemap.py#L844)
- [x] Rename confusing `StackFrame._frame_info_is_right_before_core()` (renamed in #650)
- [x] Prune unused methods in the `StackFrame - PyTealFrame - TealMapItem` hierarchy (#689)
- [x] Final status checks (ensure all changes are actually necessary) (#689)
- [x] which of the usages of `._sframes_container` are actually necessary. To see all usages: [f96ef86](https://github.com/algorand/pyteal/pull/650/commits/f96ef8657124466884974501137764a3f7c0a266)
- [x] audit the following test files and either remove, unskip, or provide clear commentary as to why the skipped test needs to stay in (#689 unified using `STABLE_SLOT_GENERATION: bool` constants currently `False):
- [x] tests/integration/sourcemap_monkey_integ_test.py
- [x] tests/unit/sourcemap_constructs311_test.py
- [x] tests/unit/sourcemap_constructs_allpy_test.py
- [x] tests/unit/sourcemap_monkey_unit_test.py
- [x] tests/unit/sourcemap_test.py
- [x] At the end of #676 pay close attention to `_PyTealSourceMap._infer()` and see which lines are still being hit. Probably, there will be some situations that are no longer needed. (#689 tightened this up)
- [ ] Refactor `NatalStackFrame` to provide a single frame instead of a list
- [ ] Coverage triage
- [ ] first item to notice: we could do better on subroutines such as the recursive fibonacci example:
- [ ] with scratch slots: https://github.com/algorand/pyteal/blob/6a49be7b4c74abe6fa08493bdb8e5585dabf0bca/tests/unit/sourcemap_constructs311_test.py#L1901
- [ ] with frame pointers: https://github.com/algorand/pyteal/blob/6a49be7b4c74abe6fa08493bdb8e5585dabf0bca/tests/unit/sourcemap_constructs311_test.py#L1955
- [ ] Pull in updated version of `mjpieters`'s **SourceMap** to get faster R3SourceMap (cf: https://github.com/tzaffi/pyteal/blob/f2a945a8eeb396e9c544b62cd3d1a6383bfc9bff/pyteal/compiler/sourcemap.py#L75)
- [ ] Investigate the feasibility of removing the `_sframes_container` property from `class TealComponent` and whenever this is needed, simply overwrite either `self.expr.stack_frames` or `self._stack_frames`. (partially removed from much of hierarchy in #689)
- [ ] If feasible, implement it
- [ ] Also consider having a new method `NatalStackFrame.spawn()` that keeps the original `NatalStackFrame` as a child.
- [ ] It's likely that `_PyTealSourceMapper` will never be exposed to the end-user. In that case, the logic for caching build items is overly complex. This should be simplified at the end or shortly after #676
- [ ] Less clunky workflow and artifact access in `_PyTealSourceMapper` (cf: https://github.com/tzaffi/pyteal/blob/f2a945a8eeb396e9c544b62cd3d1a6383bfc9bff/pyteal/compiler/sourcemap.py#L730)

## Dependencies

None

## Urgency

Medium - while the urgency of any improvement individually is Low, addressing at least a few of these issues in the short term will improve the feature's usefuleness.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.