-
Notifications
You must be signed in to change notification settings - Fork 427
Dyno: add backtracing LLDB script #26944
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Danila Fedorin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm, but I'm not familiar with the LLDB api so it's a pretty basic review.
Why not add this to compiler/etc/lldb.commands
and have it loaded in chpl --lldb
by default?
it would be of limited utility in |
Actually, because I am not sure how this will work when the switch is thrown outside of the "usual" developer environment (e.g., what if a homebrew install throws |
Signed-off-by: Danila Fedorin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
util/devel/dyno_backtrace.py:50
- [nitpick] Using magic indices for accessing frame variables (e.g., index 0, 1, and 3) makes the code less clear; consider introducing descriptive variable names or constants to document the expected variable roles.
rc = frame.variables[0]
def get_child_by_name(value, name): | ||
return next(child for child in value.children if child.name == name) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function get_child_by_name assumes that a child with the specified name exists, which may result in a StopIteration error if not found. Consider adding error handling or a default value.
def get_child_by_name(value, name): | |
return next(child for child in value.children if child.name == name) | |
def get_child_by_name(value, name, default=None): | |
try: | |
return next(child for child in value.children if child.name == name) | |
except StopIteration: | |
if default is not None: | |
return default | |
raise ValueError(f"Child with name '{name}' not found") |
Copilot uses AI. Check for mistakes.
LLDB exposes a convenient Python API. We can leverage this API to print clean stacktaces while resolving Dyno programs. See also my comment in the file.
The output looks like this:
Reviewed by @arifthpe -- thanks!