[Lldb-commits] [lldb] Colorize output when searching for symbols in lldb (PR #69422)

[David Spickett via lldb-commits]

================
@@ -247,7 +247,17 @@ class Address {
   bool Dump(Stream *s, ExecutionContextScope *exe_scope, DumpStyle style,
             DumpStyle fallback_style = DumpStyleInvalid,
             uint32_t addr_byte_size = UINT32_MAX,
-            bool all_ranges = false) const;
+            bool all_ranges = false,
+            std::vector<std::pair<bool, const char*>>* info = nullptr) const;
----------------
DavidSpickett wrote:

Gonna choose this one as the example. You're on the right lines again but implementation is a bit odd. Let's take it bit by bit.

You're using a pointer not a reference, which makes some sense as you can't default construct a reference. So just FYI in future, try to use `const &` if you are never going to pass a `nullptr` *and* can default construct the type (or better, use `optional`).

Next bit, `pair` is a decent choice but I don't see the need for a vector here. A single pair would do, so `std::pair<bool, const char*>* ` would suffice.

If you were to do that, you could default construct the parameter and pass via copy. No reference or pointer needed (and pointer/bool are small types cheap to pass by copy).

So you may choose to apply those idea maybe not after I tell you the next bit, which makes them academic.

If you were to write out a table of values of `name` (aka the pattern) and whether we're using colour, what would it look like? This:

| const char* name | use_colour | Do we use name? |
|---------------------|-------------|--------------------|
| non-null                | false           | No |
| nullptr                   | false           | No |
| non-null                | true            | Yes |
| nullptr                   | true            | No  |

(This is a https://en.wikipedia.org/wiki/Truth_table if you haven't seen one before)

Now look where we use the name. What's the equation for that state?
```
if (name is non-null) and (use_colour is true)
```
Which means that any function currently taking this pair/vector could just take `name`. Except that we change the value of name depending on the value of `use_colour`.

If `name` is non-null but colour is disabled, make `name` nullptr. The result is the same, but functions only need `name` to know what to do.

Example: https://godbolt.org/z/1W3EWTPqa

By doing this you can combine name and use_colour earlier in the callstack, and only pass name to the subsequent functions.

If we later added a way to highlight names that did not use colour, then yes, you would need a separate bool. But that is not what you're doing here.

https://github.com/llvm/llvm-project/pull/69422