Emit a multi-map of Sierra to Cairo variables in FunctionDebugInfo#9991
Emit a multi-map of Sierra to Cairo variables in FunctionDebugInfo#9991integraledelebesgue wants to merge 1 commit into
FunctionDebugInfo#9991Conversation
|
This change is |
|
This change is part of the following stack: Change managed by git-spice. |
integraledelebesgue
force-pushed
the
debugger-annotations-v2/var-multimap
branch
from
755d61f to
bda4cf2
Compare
PR SummaryMedium Risk Overview Serialization no longer builds that map only from Breaking shape for consumers of the debugger annotation: field renamed from Reviewed by Cursor Bugbot for commit caca9d7. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
integraledelebesgue
force-pushed
the
debugger-annotations-v2/var-multimap
branch
from
bda4cf2 to
abd9ac1
Compare
integraledelebesgue
force-pushed
the
debugger-annotations-v2/var-multimap
branch
from
abd9ac1 to
142b89f
Compare
piotmag769
force-pushed
the
debugger-annotations-v2/var-multimap
branch
from
142b89f to
0890266
Compare
|
|
||
| for (sierra_param, (name, span)) in params_without_builtins { | ||
| let sierra_var = SierraVarId(sierra_param.id.id); | ||
| bindings.entry(sierra_var).or_default().push((name, span)); |
There was a problem hiding this comment.
Param zip misaligns after drops
Medium Severity
Parameter mapping pairs Sierra and Cairo params by reverse zip, assuming equal-length suffixes after skipping builtins. get_function_params_as_cairo_variables can omit entries when span resolution fails, shortening the Cairo list without shortening Sierra params, so later Sierra parameters get incorrect names and spans in sierra_to_cairo_variables.
Reviewed by Cursor Bugbot for commit 0890266. Configure here.
piotmag769
force-pushed
the
debugger-annotations-v2/var-multimap
branch
from
0890266 to
caca9d7
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit caca9d7. Configure here.
| let sierra_var = SierraVarId(sierra_param.id.id); | ||
| bindings.entry(sierra_var).or_default().push((name, span)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Loop functions lose param mappings
Medium Severity
Serializable debug info no longer uses variables_locations, and parameter bindings are built only when get_function_params_as_cairo_variables can cast the signature to FunctionSignature. For loop/while/for bodies lowered to recursive Sierra functions, that cast fails, so Sierra parameter variables are never added to sierra_to_cairo_variables even though they still appear in function.params.
Reviewed by Cursor Bugbot for commit caca9d7. Configure here.
2 participants
Summary
Changed
SerializableFunctionDebugInfo's'sierra_to_cairo_variabletosierra_to_cairo_variables, which is now populated by a vector of Cairo variables.The mapping now includes entries for Sierra variables whose ID's are re-used, e.g. produced by
store_temporduplibfuncs.Treat this PR as a preparation for changing the logic of reconstructing the Sierra -> Cairo variable mapping that handles every problematic case, implemented later in the stack.
Type of change
Please check one:
Why is this change needed?
It allows the Cairo debugger to handle variables with re-used IDs.
What was the behavior or documentation before?
What is the behavior or documentation after?
Related issue or discussion (if any)
Additional context