[Lldb-commits] [PATCH] D32168: [LLDB][MIPS] Fix TestStepOverBreakpoint.py failure

Jim Ingham via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed Apr 26 11:04:44 PDT 2017


jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

The substance seems fine.

I'm not sure I would guess what GetInstructionsCount with canSetBreakpoint == true would do without reading the code.  You could fix this with a more explicit parameter name, but I can't think of a good name that's short enough not to be awful, something like: `excludeInstructionsWhereYouCantSetBreakpoints` would get it but yuck...  Better to just add a header comment explaining what that parameter actually does.

It's also odd to have the SBInstructionList hold the logic determining whether you can set a breakpoint on an instruction.  That should really be in SBInstruction, or even better should be in the lldb_private::Instruction class, since this seems like a generally useful bit of knowledge, and then routed through SBInstruction.


https://reviews.llvm.org/D32168





More information about the lldb-commits mailing list