HomePhabricator

[lldb] Make deleting frame recognizers actually work

Authored by teemperor on Jul 23 2020, 8:25 AM.

Description

[lldb] Make deleting frame recognizers actually work

Summary:

Frame recognizers are stored alongside a flag that indicates whether they were
deleted by the user. If the flag is set, they are supposed to be ignored by the
rest of the frame recognizer code. 'frame recognizer delete' is supposed to set
that flag. 'frame recognizer clear' however actually deletes all frame
recognizers (so, it doesn't set the flag but directly deletes them from the
list).

The current implementation of this concept is pretty broken. `frame recognizer
delete` sets the flag, but it somehow thinks that the recognizer id is an index
in the recognizer list. That's not true as it's actually just a member of each
recognizer entry. So it actually just sets the deleted flag for a random other
recognizer. The tests for the recognizer still pass as frame recognizer list
is also broken and just completely ignored the deleted flag and lists all
recognizers. Also frame recognizer delete just ignores if it can't actually
delete a recognizer if the id is invalid.

I think we can simplify this whole thing by just actually deleting recognizers
instead of making sure all code is actually respecting the deleted flag. I
assume the intention of this was to make sure that all recognizers are getting
unique ids over the course of an LLDB session, but as clear is actually
deleting them and we keep recycling ids, that didn't really work to begin with.

This patch deletes the deleted flag and just actually deletes the stored
recognizer. Also adds the missing error message in case it find a recognizer
with a given id.

Reviewers: mib

Reviewed By: mib

Subscribers: abidh, JDevlieghere

Differential Revision: https://reviews.llvm.org/D84404

Details