Add a comment in the SB API doc about keeping the SB API's lightweight.#108462
Merged
Add a comment in the SB API doc about keeping the SB API's lightweight.#108462
Conversation
Member
|
@llvm/pr-subscribers-lldb Author: None (jimingham) ChangesFull diff: https://github.com/llvm/llvm-project/pull/108462.diff 1 Files Affected:
diff --git a/lldb/docs/resources/sbapi.rst b/lldb/docs/resources/sbapi.rst
index cf32cc6c815581..4ca3909e0f2919 100644
--- a/lldb/docs/resources/sbapi.rst
+++ b/lldb/docs/resources/sbapi.rst
@@ -72,6 +72,17 @@ building the LLDB framework for macOS, the headers are processed with
``unifdef`` prior to being copied into the framework bundle to remove macros
involving SWIG.
+Another good principle when adding SB API methods is: if you find yourself
+implementing a significant algorithm in the SB API method, you should not do
+that, but instead look for and then add it - if not found - as a method in the
+underlying lldb_private class, and then call that from your SB API method.
+If it was a useful algorithm, it's very likely it already exists
+because the lldb_private code also needed to do it. And if it doesn't at
+present, if it was a useful thing to do, it's likely someone will later need
+it in lldb_private and then we end up with two implementations of the same
+algorithm. If we keep the SB API code to just what's needed to manage the SB
+objects and requests, we won't get into this situation.
+
Lifetime
--------
Many SB API methods will return strings in the form of ``const char *`` values.
|
jeffreytan81
approved these changes
Sep 13, 2024
Contributor
jeffreytan81
left a comment
There was a problem hiding this comment.
Now I wish we can feed this kind of information to AI and let AI code review and catch it :-)
medismailben
approved these changes
Sep 13, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.