-
Notifications
You must be signed in to change notification settings - Fork 15.5k
[lldb][API] Make SB-API functions const if possible. #172687
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
base: main
Are you sure you want to change the base?
Conversation
Only applied to functions that were added after 22.x tag.
|
@llvm/pr-subscribers-lldb Author: Ebuka Ezike (da-viper) ChangesOnly applied to functions that were added after 22.x tag. Full diff: https://github.com/llvm/llvm-project/pull/172687.diff 13 Files Affected:
diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h
index 7a08a08262207..39980349065cf 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -360,7 +360,7 @@ class LLDB_API SBDebugger {
const char *arch);
/// Find a target with the specified unique ID.
- lldb::SBTarget FindTargetByGloballyUniqueID(lldb::user_id_t id);
+ lldb::SBTarget FindTargetByGloballyUniqueID(lldb::user_id_t id) const;
/// Get the number of targets in the debugger.
uint32_t GetNumTargets();
diff --git a/lldb/include/lldb/API/SBMemoryRegionInfo.h b/lldb/include/lldb/API/SBMemoryRegionInfo.h
index dc5aa0858e1e3..3ff9b6beff20c 100644
--- a/lldb/include/lldb/API/SBMemoryRegionInfo.h
+++ b/lldb/include/lldb/API/SBMemoryRegionInfo.h
@@ -126,7 +126,7 @@ class LLDB_API SBMemoryRegionInfo {
///
/// The description format is: [Hex start - Hex End) with associated
/// permissions (RWX)
- bool GetDescription(lldb::SBStream &description);
+ bool GetDescription(lldb::SBStream &description) const;
private:
friend class SBProcess;
diff --git a/lldb/include/lldb/API/SBModuleSpec.h b/lldb/include/lldb/API/SBModuleSpec.h
index b80a52b7a235f..0e7f0f3489596 100644
--- a/lldb/include/lldb/API/SBModuleSpec.h
+++ b/lldb/include/lldb/API/SBModuleSpec.h
@@ -87,7 +87,7 @@ class LLDB_API SBModuleSpec {
bool GetDescription(lldb::SBStream &description);
- lldb::SBTarget GetTarget();
+ lldb::SBTarget GetTarget() const;
/// Set the target to be used when resolving a module.
///
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h
index 7b05377966965..5f5f99a46f206 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -84,7 +84,7 @@ class LLDB_API SBSaveCoreOptions {
///
/// \return
/// The set process, or an invalid SBProcess if no process is set.
- SBProcess GetProcess();
+ SBProcess GetProcess() const;
/// Add a thread to save in the core file.
///
diff --git a/lldb/include/lldb/API/SBSymbol.h b/lldb/include/lldb/API/SBSymbol.h
index 580458ede212d..3ec0dab5112f5 100644
--- a/lldb/include/lldb/API/SBSymbol.h
+++ b/lldb/include/lldb/API/SBSymbol.h
@@ -91,7 +91,7 @@ class LLDB_API SBSymbol {
///
/// \returns
/// Returns the ID of this symbol.
- uint32_t GetID();
+ uint32_t GetID() const;
bool operator==(const lldb::SBSymbol &rhs) const;
@@ -108,7 +108,7 @@ class LLDB_API SBSymbol {
bool IsSynthetic();
/// Returns true if the symbol is a debug symbol.
- bool IsDebug();
+ bool IsDebug() const;
/// Get the string representation of a symbol type.
static const char *GetTypeAsString(lldb::SymbolType symbol_type);
diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h
index 0318492f1054c..dd2cf59b831da 100644
--- a/lldb/include/lldb/API/SBTarget.h
+++ b/lldb/include/lldb/API/SBTarget.h
@@ -338,7 +338,7 @@ class LLDB_API SBTarget {
/// \return
/// A lldb::SBModule object that represents the found module, or an
/// invalid SBModule object if no module was found.
- lldb::SBModule FindModule(const lldb::SBModuleSpec &module_spec);
+ lldb::SBModule FindModule(const lldb::SBModuleSpec &module_spec) const;
/// Find compile units related to *this target and passed source
/// file.
@@ -359,7 +359,7 @@ class LLDB_API SBTarget {
const char *GetTriple();
- const char *GetArchName();
+ const char *GetArchName() const;
const char *GetABIName();
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index 697549706ed07..5be7b8c45efd5 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -37,7 +37,7 @@ class SaveCoreOptions {
const std::optional<lldb_private::FileSpec> GetOutputFile() const;
Status SetProcess(lldb::ProcessSP process_sp);
- lldb::ProcessSP GetProcess() { return m_process_sp; }
+ lldb::ProcessSP GetProcess() const { return m_process_sp; }
Status AddThread(lldb::ThreadSP thread_sp);
bool RemoveThread(lldb::ThreadSP thread_sp);
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 38d21241cbea3..e97fdd0e352f2 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -937,7 +937,7 @@ uint32_t SBDebugger::GetIndexOfTarget(lldb::SBTarget target) {
return m_opaque_sp->GetTargetList().GetIndexOfTarget(target.GetSP());
}
-SBTarget SBDebugger::FindTargetByGloballyUniqueID(lldb::user_id_t id) {
+SBTarget SBDebugger::FindTargetByGloballyUniqueID(lldb::user_id_t id) const {
LLDB_INSTRUMENT_VA(this, id);
SBTarget sb_target;
if (m_opaque_sp) {
diff --git a/lldb/source/API/SBMemoryRegionInfo.cpp b/lldb/source/API/SBMemoryRegionInfo.cpp
index cd25be5d52769..1c0de6aece097 100644
--- a/lldb/source/API/SBMemoryRegionInfo.cpp
+++ b/lldb/source/API/SBMemoryRegionInfo.cpp
@@ -160,7 +160,7 @@ int SBMemoryRegionInfo::GetPageSize() {
return m_opaque_up->GetPageSize();
}
-bool SBMemoryRegionInfo::GetDescription(SBStream &description) {
+bool SBMemoryRegionInfo::GetDescription(SBStream &description) const {
LLDB_INSTRUMENT_VA(this, description);
Stream &strm = description.ref();
diff --git a/lldb/source/API/SBModuleSpec.cpp b/lldb/source/API/SBModuleSpec.cpp
index 031ba1256d18a..8371106dcbf9e 100644
--- a/lldb/source/API/SBModuleSpec.cpp
+++ b/lldb/source/API/SBModuleSpec.cpp
@@ -175,7 +175,7 @@ void SBModuleSpec::SetObjectSize(uint64_t object_size) {
m_opaque_up->SetObjectSize(object_size);
}
-SBTarget SBModuleSpec::GetTarget() {
+SBTarget SBModuleSpec::GetTarget() const {
LLDB_INSTRUMENT_VA(this);
return SBTarget(m_opaque_up->GetTargetSP());
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp b/lldb/source/API/SBSaveCoreOptions.cpp
index e8b81ee57f5a9..54be64a9cc5a8 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -81,7 +81,7 @@ SBError SBSaveCoreOptions::SetProcess(lldb::SBProcess process) {
return m_opaque_up->SetProcess(process.GetSP());
}
-SBProcess SBSaveCoreOptions::GetProcess() {
+SBProcess SBSaveCoreOptions::GetProcess() const {
LLDB_INSTRUMENT_VA(this);
return SBProcess(m_opaque_up->GetProcess());
}
diff --git a/lldb/source/API/SBSymbol.cpp b/lldb/source/API/SBSymbol.cpp
index 3030c83292127..73f082ebde6a3 100644
--- a/lldb/source/API/SBSymbol.cpp
+++ b/lldb/source/API/SBSymbol.cpp
@@ -202,12 +202,12 @@ SymbolType SBSymbol::GetType() {
return eSymbolTypeInvalid;
}
-uint32_t SBSymbol::GetID() {
+uint32_t SBSymbol::GetID() const {
LLDB_INSTRUMENT_VA(this);
if (m_opaque_ptr)
return m_opaque_ptr->GetID();
- return 0;
+ return UINT32_MAX;
}
bool SBSymbol::IsExternal() {
@@ -226,7 +226,7 @@ bool SBSymbol::IsSynthetic() {
return false;
}
-bool SBSymbol::IsDebug() {
+bool SBSymbol::IsDebug() const {
LLDB_INSTRUMENT_VA(this);
if (m_opaque_ptr)
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index 78c2d49d647b5..99dfbb3fd9bce 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -1581,7 +1581,7 @@ SBModule SBTarget::FindModule(const SBFileSpec &sb_file_spec) {
return sb_module;
}
-SBModule SBTarget::FindModule(const SBModuleSpec &sb_module_spec) {
+SBModule SBTarget::FindModule(const SBModuleSpec &sb_module_spec) const {
LLDB_INSTRUMENT_VA(this, sb_module_spec);
SBModule sb_module;
@@ -1624,7 +1624,7 @@ const char *SBTarget::GetTriple() {
return nullptr;
}
-const char *SBTarget::GetArchName() {
+const char *SBTarget::GetArchName() const {
LLDB_INSTRUMENT_VA(this);
if (TargetSP target_sp = GetSP()) {
|
|
Can you verify that none of these methods were cherry-picked to 21.x? If they were, this change would be an ABI break. Side note: Practically speaking this change doesn't mean that much. There's not a huge difference between a const and non-const SB object since they're opaque. On principle though I do like this kind of change. |
built all three branches and stripped the dynamic symbols using # awk get the last column and reprove the suffix '@.*'
nm -D ./lib/liblldb.so.21.1.0 | awk '{ symbol = $NF; sub(/@.*/, "", symbol); print symbol }' | c++filt | grep -i lldb::SB | sort > ~/logs/21.1.logand diffed it for missing symbols. For completeness I attached the logs. 22.0.log |
bulbazord
left a comment
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.
I'll trust your analysis of which methods can be made const.
| if (m_opaque_ptr) | ||
| return m_opaque_ptr->GetID(); | ||
| return 0; | ||
| return UINT32_MAX; |
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.
Why did this change?
medismailben
left a comment
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.
As much as I like this, technically this is ABI breaking since the mangling of those method will change.
The best you can do here is have overloads for each non-const method and mark the old methods deprecated.
@medismailben I assume we is guarantee stable ABI for only released versions. And the changed function are for only newly added functions in 22.git branch that has not been released yet. |
Let's see if this causes any breakages then :) |
Only applied to functions that were added after 22.x tag.