Skip to content

Conversation

@mbucko
Copy link
Contributor

@mbucko mbucko commented Oct 29, 2024

No description provided.

@mbucko mbucko requested a review from JDevlieghere as a code owner October 29, 2024 20:15
@mbucko mbucko requested a review from clayborg October 29, 2024 20:15
@llvmbot llvmbot added the lldb label Oct 29, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 29, 2024

@llvm/pr-subscribers-lldb

Author: Miro Bucko (mbucko)

Changes

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

1 Files Affected:

  • (modified) lldb/source/Core/PluginManager.cpp (+1-1)
diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp
index a5219025495a91..50bbfba33ee512 100644
--- a/lldb/source/Core/PluginManager.cpp
+++ b/lldb/source/Core/PluginManager.cpp
@@ -209,7 +209,7 @@ template <typename Instance> class PluginInstances {
     Instance instance =
         Instance(name, description, callback, std::forward<Args>(args)...);
     m_instances.push_back(instance);
-    return false;
+    return true;
   }
 
   bool UnregisterPlugin(typename Instance::CallbackType callback) {

Comment on lines 209 to 211
Instance instance =
Instance(name, description, callback, std::forward<Args>(args)...);
m_instances.push_back(instance);
Copy link
Member

Choose a reason for hiding this comment

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

While you're here... :-) Should this use emplace_back to create the instance object in place?

@jimingham
Copy link
Collaborator

We obviously aren't checking this return anywhere. The only way it can return false that I can see is if you call RegisterPlugin with no callback pointer - which seems more like a programming error than a run-time check. Do we actually need this bool return?

@mbucko
Copy link
Contributor Author

mbucko commented Nov 25, 2024

We obviously aren't checking this return anywhere. The only way it can return false that I can see is if you call RegisterPlugin with no callback pointer - which seems more like a programming error than a run-time check. Do we actually need this bool return?

It is indeed being checked in many places, I tried removing it and the compiler was not happy :)

@mbucko mbucko closed this Nov 25, 2024
@mbucko mbucko reopened this Nov 26, 2024
@mbucko mbucko merged commit 86f7f08 into llvm:main Nov 26, 2024
12 checks passed
@mbucko mbucko deleted the register_fix branch November 26, 2024 19:05
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.

5 participants