jingham added a comment.
This is good. The addition of the "info" command will be helpful for people
trying to debug their recognizers. It's okay to add multiple -s and -n's later
- though the fact that you don't allow "apply to all frames" may make us want
the ability to provide more than one option sooner rather than later...
Sorry for this thought coming late, but I worry a little bit about the fact
that the class name is the recognizers identity, particularly for deleting. I
might have a good recognizer class that I want to add to one library, then
later in the session want to apply it to another library as well. The second
addition will mean that the name now exists twice but with different shared
libraries, and then since I only provide the name to "delete" I can't tell
which one I've deleted.
It also makes the API's a little odd, since the name of the recognizer is
important to the commands for handling it, but it isn't clear from the API's
that I have to provide a unique name for them.
I think it would be better to have the RecognizerManager keep an ID for each
recognizer, and have list print and delete take the index.
Also, it might be convenient to have "frame recognizer delete" with no
arguments query "Do you want to delete all recognizers" and then delete them.
That's the way "break delete" works so it's an accepted pattern in lldb. This
could be done as a follow-on, however.
================
Comment at:
packages/Python/lldbsuite/test/functionalities/frame-recognizer/TestFrameRecognizer.py:54
+
+ self.runCmd("b foo")
+ self.runCmd("r")
----------------
Would you mind using:
lldbutil.run_break_set_by_symbol(self, "foo")
instead of this runCmd. The lldbutil routine will check that the breakpoint
got set and error out here if it didn't. That will reduce the head-scratching
if for some reason we fail to set the breakpoint and then the test falls over
downstream.
================
Comment at:
packages/Python/lldbsuite/test/functionalities/frame-recognizer/TestFrameRecognizer.py:91
+ """
+ self.runCmd("b bar")
+ self.runCmd("c")
----------------
Also here.
https://reviews.llvm.org/D44603
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
