Skip to content

Conversation

@barsolo2000
Copy link
Contributor

Updated SBMemoryRegionInfo doc strings:

  • Added detailed documentation for the GetDescription() method.
  • Included information about the overwritten len() and str() functions for the SBMemoryRegionInfo type, explaining their behavior and usage.

@github-actions
Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot llvmbot added the lldb label Jul 21, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 21, 2025

@llvm/pr-subscribers-lldb

Author: None (barsolo2000)

Changes

Updated SBMemoryRegionInfo doc strings:

  • Added detailed documentation for the GetDescription() method.
  • Included information about the overwritten len() and str() functions for the SBMemoryRegionInfo type, explaining their behavior and usage.

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

1 Files Affected:

  • (modified) lldb/bindings/interface/SBMemoryRegionInfoDocstrings.i (+36-20)
diff --git a/lldb/bindings/interface/SBMemoryRegionInfoDocstrings.i b/lldb/bindings/interface/SBMemoryRegionInfoDocstrings.i
index d7c68baf100e2..6b7f4a32ed17e 100644
--- a/lldb/bindings/interface/SBMemoryRegionInfoDocstrings.i
+++ b/lldb/bindings/interface/SBMemoryRegionInfoDocstrings.i
@@ -1,31 +1,47 @@
-%feature("docstring",
-"API clients can get information about memory regions in processes."
-) lldb::SBMemoryRegionInfo;
+% feature("docstring",
+          "API clients can get information about memory regions in processes.
+
+          When printed using str(), the memory region info is formatted as:
+                '[Hex start - Hex End] RWX' ")
+        lldb::SBMemoryRegionInfo;
 
 %feature("docstring", "
         Returns whether this memory region has a list of modified (dirty)
         pages available or not.  When calling GetNumDirtyPages(), you will
-        have 0 returned for both \"dirty page list is not known\" and 
+        have 0 returned for both \"dirty page list is not known\" and
         \"empty dirty page list\" (that is, no modified pages in this
         memory region).  You must use this method to disambiguate."
 ) lldb::SBMemoryRegionInfo::HasDirtyMemoryPageList;
 
-%feature("docstring", "
-        Return the number of dirty (modified) memory pages in this
-        memory region, if available.  You must use the 
-        SBMemoryRegionInfo::HasDirtyMemoryPageList() method to
-        determine if a dirty memory list is available; it will depend
-        on the target system can provide this information."
-) lldb::SBMemoryRegionInfo::GetNumDirtyPages;
+% feature(
+      "docstring",
+      "
+      Return the number of dirty(modified) memory pages in this memory region,
+      if available.You must use the SBMemoryRegionInfo::HasDirtyMemoryPageList()
+          method to determine if a dirty memory list is available;
+      it will depend on the target system can provide this information."
+      ) lldb::SBMemoryRegionInfo::GetNumDirtyPages;
 
-%feature("docstring", "
-        Return the address of a modified, or dirty, page of memory.
-        If the provided index is out of range, or this memory region 
-        does not have dirty page information, LLDB_INVALID_ADDRESS 
-        is returned."
-) lldb::SBMemoryRegionInfo::GetDirtyPageAddressAtIndex;
+% feature("docstring",
+          "
+          Return the address of a modified,
+          or dirty, page of memory.If the provided index is out of range,
+          or this memory region does not have dirty page information,
+          LLDB_INVALID_ADDRESS is returned."
+          ) lldb::SBMemoryRegionInfo::GetDirtyPageAddressAtIndex;
+
+% feature("docstring", "
+                       Return the size of pages in this memory region .0 will be
+                           returned if this information was unavailable."
+          ) lldb::SBMemoryRegionInfo::GetPageSize();
 
 %feature("docstring", "
-        Return the size of pages in this memory region.  0 will be returned
-        if this information was unavailable."
-) lldb::SBMemoryRegionInfo::GetPageSize();
+        takes a SBStream parameter 'description' where it will write the output to.
+        The function will formats the memory region information into a string with Memory region info
+        [Hex start - Hex End) and premission flags R/W/X.
+        Returns a boolean value indicating success or failure.
+
+        alternatively, to get the size of the memory region is to use
+        the len() function on the SBMemoryRegionInfo object.
+        The function function will return the size of the memory region"
+) lldb::SBMemoryRegionInfo::GetDescription;

@JDevlieghere
Copy link
Member

Now that we include Doxygen comments in the docstring (#147617), should we move these into the C++ header?

@Jlalond
Copy link
Contributor

Jlalond commented Jul 21, 2025

@JDevlieghere How would we go about documenting some of the python methods len() and str() in doxygen? Asking for myself if you have an example

@JDevlieghere
Copy link
Member

@JDevlieghere How would we go about documenting some of the python methods len() and str() in doxygen? Asking for myself if you have an example

Since those are Python specific they should remain docstrings in the .i interface files. I expect the latter to stick around, for that reason or when we have comments that are more tailored towards Python. But generic comments should go in the headers as Doxygen comments so both C++ and Python users can benefit from them. Unfortunately there's also no automatic way to convert them to Lua documentation, but maybe that'll come in a future version of SWIG.

@Jlalond
Copy link
Contributor

Jlalond commented Jul 23, 2025

@JDevlieghere When you get a chance, is this the layout you envisioned for splitting Python and C++ comments?

@github-actions
Copy link

github-actions bot commented Jul 23, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@Jlalond Jlalond left a comment

Choose a reason for hiding this comment

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

LGTM

@Jlalond Jlalond merged commit 3d098f7 into llvm:main Aug 1, 2025
9 checks passed
@github-actions
Copy link

github-actions bot commented Aug 1, 2025

@barsolo2000 Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

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.

4 participants