Fix BroadcasterManager::RemoveListener to really remove the listener

[rnk]

Summary

This appears to be a real bug caught by -Wunused-value. std::find_if
doesn't modify the underlying collection, it just returns an iterator
pointing to the matching element.

[merge_guards_bot]

Unit tests: unknown.

clang-tidy: fail. clang-tidy found 1 errors and 0 warnings. 0 of them are added as review comments below (why?).

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[JDevlieghere]

Thanks for fixing this, Reid!

[amccarth]

Wow, cool bug.

It's too bad the original code re-used an iterator variable instead of make a new name (which would have helped the compiler find the problem). Note that the one they used is shadowed just a couple lines later.

It's too bad the original code feels it's necessary to create iter and end_iter up front. I know raw loops require this trick to avoid re-computing the end iterator on each iteration through the loop, but that shouldn't be necessary on algorithms like find_if.

It's too bad that erase with an end iterator isn't just a safe no-op, so that zillions of callers aren't required to check find's return value. Without the visual noise, it would be easier to write exactly what you want.

It's too bad the compiler cannot recognize that find_if has no side effects and thus ignoring its return value makes the statement a no-op.

It's too bad the std::erase(std::remove_if(...)) idiom is so cumbersome. I realize that would likely be overkill here, since you apparently want to erase just the first one that matches the predicate. Nonetheless, it would be harder to make this kind of bug.

lldb/source/Utility/Broadcaster.cpp

 }
 
 void BroadcasterManager::RemoveListener(Listener *listener) {
   std::lock_guard<std::recursive_mutex> guard(m_manager_mutex);
   ListenerMatchesPointer predicate(listener);
   listener_collection::iterator iter = m_listeners.begin(),
                                 end_iter = m_listeners.end();
 
-  std::find_if(iter, end_iter, predicate);
+  iter = std::find_if(iter, end_iter, predicate);
   if (iter != end_iter)
     m_listeners.erase(iter);
 
   while (true) {
     collection::iterator iter, end_iter = m_event_map.end();
     iter = find_if(m_event_map.begin(), end_iter, predicate);
     if (iter == end_iter)
       break;