Conversation
|
|
|
Checking stubs for changes and corresponding version bump in origin/feat/arc56_source_info Last puyapy release: v5.7.1 💡 Stub version change: 3.4.0 -> 3.5.0 |
|
joe-p
force-pushed
the
feat/arc56_source_info
branch
3 times, most recently
from
113a2fa to
5f1d58a
Compare
|
joe-p
force-pushed
the
feat/arc56_source_info
branch
2 times, most recently
from
9bd17e5 to
c9d370a
Compare
joe-p
force-pushed
the
feat/arc56_source_info
branch
from
c9d370a to
fcdb532
Compare
There was a problem hiding this comment.
Pull request overview
Adds richer ARC-56 source mappings to help developers debug runtime errors by including TEAL line numbers and high-level source references where available.
Changes:
- Threads a new
full_source_infooption from CLI/options through compilation and ARC-56 generation. - Extends ARC-56
SourceInfowith optionaltealandsourcefields and generates these mappings. - Updates ARC-56 action-combining logic to validate/filter OnComplete actions.
Reviewed changes
Copilot reviewed 7 out of 441 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/puyapy/compile.py | Forces full_source_info off for ARC4 client generation path. |
| src/puyapy/main.py | Adds CLI flag/default for full_source_info and documents it. |
| src/puya/service.py | Passes full_source_info through service compilation into ARC-56 generation. |
| src/puya/options.py | Adds full_source_info option (default enabled). |
| src/puya/compile.py | Passes full_source_info through artifact writing into ARC-56 generation. |
| src/puya/arc56_models.py | Extends ARC-56 source info model to include TEAL/source references. |
| src/puya/arc56.py | Implements decoding/mapping logic and plumbs full_source_info through ARC-56 output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for line_num, line in enumerate(teal_src.splitlines(), start=1): | ||
| stripped = line.strip() | ||
| if not stripped or stripped.startswith(("//", "#pragma")) or stripped.endswith(":"): | ||
| continue | ||
| op_line_numbers.append((line_num, stripped)) | ||
|
|
||
| if not op_line_numbers: | ||
| return {} | ||
|
|
||
| line_idx = 0 | ||
| result = dict[int, int]() | ||
| for pc in sorted(pc_events): | ||
| event = pc_events[pc] | ||
| op = event.get("op") if event else None | ||
| if not op: | ||
| continue | ||
| while line_idx < len(op_line_numbers): | ||
| line_num, line_op = op_line_numbers[line_idx] | ||
| line_idx += 1 | ||
| if line_op == op: | ||
| result[pc] = line_num | ||
| break | ||
| return result |
There was a problem hiding this comment.
TEAL line-to-op matching is comparing the full stripped TEAL line (which often includes operands and inline comments, e.g. int 1, bnz label, int 1 // comment) to event['op'] (likely just the opcode mnemonic, e.g. int, bnz). This will fail to match for most ops and produce incorrect/missing teal mappings. Parse the first token as the opcode (and strip inline comments, and treat # comment lines similarly to //) before comparing to op.
| def _base64vlq_decode(value: str) -> tuple[int, int, int, int]: | ||
| decoded = [] | ||
| current = 0 | ||
| shift = 0 | ||
| for char in value: | ||
| digit = _decode_base64_char(char) | ||
| continuation = digit & 0b100000 | ||
| payload = digit & 0b011111 | ||
| current |= payload << shift | ||
| shift += 5 | ||
| if continuation: | ||
| continue | ||
| sign = -1 if current & 1 else 1 | ||
| decoded.append(sign * (current >> 1)) | ||
| current = 0 | ||
| shift = 0 | ||
| if len(decoded) != 4: | ||
| raise ValueError(f"Invalid source map segment: {value!r}") | ||
| return typing.cast(tuple[int, int, int, int], tuple(decoded)) |
There was a problem hiding this comment.
Hard-failing when a VLQ segment doesn't decode to exactly 4 values can break ARC-56 generation if the mapping format varies (segments can be variable-length in common sourcemap/VLQ encodings). Consider decoding more defensively (e.g., accept 4+ values and ignore extras, or skip segments that don't match expectations) and ensure _decode_source_mappings does not propagate a ValueError that prevents emitting ARC-56.
| optimization_level: Set optimization level of output TEAL / AVM bytecode | ||
| full_source_info: Include detailed source mappings (TEAL line numbers and | ||
| source file references) in ARC-56 output | ||
| optimization_level: Set optimization level of output TEAL / AVM bytecode |
There was a problem hiding this comment.
The docstring lists optimization_level twice (lines 131 and 134). Removing the duplicate entry will prevent confusion and keep the CLI docs consistent.
| optimization_level: Set optimization level of output TEAL / AVM bytecode |
| clear_program=artifact.clear_program, | ||
| metadata=artifact.metadata, | ||
| template_prefix=template_prefix, | ||
| full_source_info=False, |
There was a problem hiding this comment.
This hard-codes full_source_info=False on the ARC-56 generation path used by write_arc4_clients, which conflicts with the new default of enabling full source info elsewhere (CLI/options default True). Consider threading the user-configured value through here (or leaving it to the ARC-56 default) so the feature behaves consistently across entry points.
| full_source_info=False, |
| create = sorted( | ||
| {oca for action in actions for oca in action.create if allowed_create_oca(oca)} | ||
| ) | ||
| call = sorted({oca for action in actions for oca in action.call if allowed_call_oca(oca)}) | ||
| return models.MethodActions( | ||
| create=sorted(set(itertools.chain.from_iterable(a.create for a in actions))), | ||
| call=sorted(set(itertools.chain.from_iterable(a.call for a in actions))), | ||
| create=typing.cast( | ||
| Sequence[typing.Literal["NoOp", "OptIn", "DeleteApplication"]], | ||
| create, | ||
| ), | ||
| call=typing.cast( | ||
| Sequence[ | ||
| typing.Literal[ | ||
| "NoOp", "OptIn", "CloseOut", "UpdateApplication", "DeleteApplication" | ||
| ] | ||
| ], | ||
| call, | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Invalid/unknown OnComplete action values are silently dropped by the allowed_*_oca filters. If an invalid value appears upstream, silently omitting it can make ARC-56 output inaccurate and harder to debug. Consider raising an error (or at least logging) when an unexpected OCA is encountered, instead of filtering it out without notification.
Adds full source information to the generated ARC56. This is generally useful for developers who need to debug unexpected errors, especially when it needs to be done by hand.
One thing to note here is that Puya doesn't seem to have a full mapping of every PC to the high-level source. Presumably this is because of the optimization pipeline. I think this is acceptable because it still gives you the TEAL, which will be still be very helpful compared to just the PC
Note: This was one-shotted by 5.3 codex. I haven't carefully reviewed the code yet