DavidSpickett added a comment.

I agree that this change names - print - restore names is going to cause 
problems. Pavel is right that this should happen in some dump function 
somewhere.

But, assuming what you've got works to an extent, you should add some testing 
first. That will allow you to refactor with less risk. You'll be able to reuse 
a lot of the tests later, so it's not wasted effort and it'll flush out some 
corner cases you haven't thought of.

For how to test this I would look to the other commands that have colour 
(`frame` was one I think) and anything interacting with the colour settings.
(if you want a cheeky way to find the tests, delete some of the colouring code 
and run the test suite - whatever fails is going to be interesting to you)



================
Comment at: lldb/source/Commands/CommandObjectTarget.cpp:1514
+  for (size_t i = 0; i < positions.size(); ++i) {
+    if (pos >= positions[i].first && pos <= positions[i].second) {
+      return true;
----------------
Doesn't seem like you need to enumerate them, so this could be:
```
for (const auto &p : positions)
   if (pos >= ....)
       return true;
return false;
```
(https://en.cppreference.com/w/cpp/language/range-for)

Or `std::find(..., <lambda>) != positions.end()` but same thing really.

Basically prefer iterators unless `i` is needed for something other than 
indexing.

If you do need the index but don't want the hassle of handling it yourself, 
there is an `llvm::enumerate` in `llvm/include/llvm/ADT/STLExtras.h` which acts 
like Python's `enumerate`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136462/new/

https://reviews.llvm.org/D136462

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to