Skip to content

Conversation

@jasonmolenda
Copy link
Collaborator

SBFrame::GetRegisters() (and the .registers / .regs extensions in Python) returns an array of register-set's, not registers like you might expect from the API name. Document this.

SBFrame::GetRegisters() (and the .registers / .regs extensions in
Python) returns an array of register-set's, not registers like you
might expect from the API name.  Document this.
@jasonmolenda
Copy link
Collaborator Author

not wedded to the specific words I used here, if anyone has an opinion. just want to get something in there.

@llvmbot
Copy link
Member

llvmbot commented Feb 6, 2025

@llvm/pr-subscribers-lldb

Author: Jason Molenda (jasonmolenda)

Changes

SBFrame::GetRegisters() (and the .registers / .regs extensions in Python) returns an array of register-set's, not registers like you might expect from the API name. Document this.


Full diff: https://github.com/llvm/llvm-project/pull/125969.diff

2 Files Affected:

  • (modified) lldb/bindings/interface/SBFrameDocstrings.i (+16)
  • (modified) lldb/bindings/interface/SBFrameExtensions.i (+2-2)
diff --git a/lldb/bindings/interface/SBFrameDocstrings.i b/lldb/bindings/interface/SBFrameDocstrings.i
index 05a876a685a9127..5823d332c4cdd63 100644
--- a/lldb/bindings/interface/SBFrameDocstrings.i
+++ b/lldb/bindings/interface/SBFrameDocstrings.i
@@ -78,6 +78,22 @@ See also SBThread."
     See also GetFunctionName()."
 ) lldb::SBFrame::IsInlined;
 
+%feature("docstring", "
+    Returns an SBValueList which is an array of one or more register
+    sets that exist for this thread.  
+    Each SBValue in the SBValueList represents one register-set. 
+    The first register-set will be the general purpose registers -- 
+    the registers printed by the `register read` command-line lldb, with 
+    no additional arguments.  
+    The register-set SBValue will have a name, e.g. 
+    SBFrame::GetRegisters().GetValueAtIndex(0).GetName() 
+    may be 'General Purpose Registers', but the general purpose 
+    register-set may not use that exact name, it is only a convention 
+    used by some stubs.
+    A register-set SBValue will have children, one child per register 
+    in the register-set."
+) lldb::SBFrame::GetRegisters;
+
 %feature("docstring", "
     Return true if this frame is artificial (e.g a frame synthesized to
     capture a tail call). Local variables may not be available in an artificial
diff --git a/lldb/bindings/interface/SBFrameExtensions.i b/lldb/bindings/interface/SBFrameExtensions.i
index e0472280666ab9c..5c7a63ac53b3ec1 100644
--- a/lldb/bindings/interface/SBFrameExtensions.i
+++ b/lldb/bindings/interface/SBFrameExtensions.i
@@ -87,8 +87,8 @@ STRING_EXTENSION_OUTSIDE(SBFrame)
         args = property(get_arguments, None, doc='''A read only property that returns a list() that contains a collection of lldb.SBValue objects that represent the argument variables in this stack frame.''')
         arguments = property(get_arguments, None, doc='''A read only property that returns a list() that contains a collection of lldb.SBValue objects that represent the argument variables in this stack frame.''')
         statics = property(get_statics, None, doc='''A read only property that returns a list() that contains a collection of lldb.SBValue objects that represent the static variables in this stack frame.''')
-        registers = property(GetRegisters, None, doc='''A read only property that returns a list() that contains a collection of lldb.SBValue objects that represent the CPU registers for this stack frame.''')
-        regs = property(GetRegisters, None, doc='''A read only property that returns a list() that contains a collection of lldb.SBValue objects that represent the CPU registers for this stack frame.''')
+        registers = property(GetRegisters, None, doc='''A read only property that returns a list() of register sets for this thread.  See SBFrame::GetRegisters() for details.''')
+        regs = property(GetRegisters, None, doc='''A read only property that returns a list() of register sets for this thread.  See SBFrame::GetRegisters() for details.''')
         register = property(get_registers_access, None, doc='''A read only property that returns an helper object providing a flattened indexable view of the CPU registers for this stack frame.''')
         reg = property(get_registers_access, None, doc='''A read only property that returns an helper object providing a flattened indexable view of the CPU registers for this stack frame''')
         parent = property(get_parent_frame, None, doc='''A read only property that returns the parent (caller) frame of the current frame.''')

Copy link
Collaborator

@jimingham jimingham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with or without my nitpicking...

statics = property(get_statics, None, doc='''A read only property that returns a list() that contains a collection of lldb.SBValue objects that represent the static variables in this stack frame.''')
registers = property(GetRegisters, None, doc='''A read only property that returns a list() that contains a collection of lldb.SBValue objects that represent the CPU registers for this stack frame.''')
regs = property(GetRegisters, None, doc='''A read only property that returns a list() that contains a collection of lldb.SBValue objects that represent the CPU registers for this stack frame.''')
registers = property(GetRegisters, None, doc='''A read only property that returns a list() of register sets for this thread. See SBFrame::GetRegisters() for details.''')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Being incredibly picky, I would say "returns the register sets for this thread as a list()". It always returns all the register sets available, which this wording makes a little more clear.

But that's being really picky.

Copy link
Contributor

@felipepiovezan felipepiovezan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for improving this, this has fooled me once before 😅

Copy link
Contributor

@hawkinsw hawkinsw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hope that those little tweaks help! I appreciate the added documentation!

@jasonmolenda
Copy link
Collaborator Author

Hope that those little tweaks help! I appreciate the added documentation!

Great suggestions, thanks!

@jasonmolenda jasonmolenda merged commit 18bd118 into llvm:main Feb 6, 2025
7 checks passed
@jasonmolenda jasonmolenda deleted the eng/document-sbframe-getregisters branch February 6, 2025 17:19
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
SBFrame::GetRegisters() (and the .registers / .regs extensions in
Python) returns an array of register-set's, not registers like you might
expect from the API name. Document this.

---------

Co-authored-by: Will Hawkins <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants