[Lldb-commits] [lldb] [lldb] colorize symbols in image lookup with a regex pattern (PR #69422)

[via lldb-commits]

=?utf-8?q?José?= L. Junior <josejunior at 10xengineers.ai>,taalhaataahir0102
 <23100293 at lums.edu.pk>,taalhaataahir0102 <23100293 at lums.edu.pk>,taalhaataahir0102
 <23100293 at lums.edu.pk>,taalhaataahir0102 <23100293 at lums.edu.pk>,
=?utf-8?q?José?= L. Junior <josejunior at 10xengineers.ai>,
=?utf-8?q?José?= L. Junior <josejunior at 10xengineers.ai>,
=?utf-8?q?José?= L. Junior <josejunior at 10xengineers.ai>,
=?utf-8?q?José?= L. Junior <josejunior at 10xengineers.ai>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/69422 at github.com>


================
@@ -246,8 +246,8 @@ class Address {
   /// \see Address::DumpStyle
   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;
+            uint32_t addr_byte_size = UINT32_MAX, bool all_ranges = false,
+            const char *pattern = nullptr) const;
----------------
taalhaataahir0102 wrote:

Hi @DavidSpickett! Sorry for the delay 🙏

`test_func3(strm, std::move(regex2))` works but it gives the following problem. 
When we use `std::move(regex2)` to pass it to `test_func3`, it's essentially moved, and after the move, the `regex2` in `LookupSymbolInModule` becomes garbage.

So in functions where pattern is moved multiple times like this:

```
static void DumpAddress(ExecutionContextScope *exe_scope,
                        const Address &so_addr, bool verbose, bool all_ranges,
                        Stream &strm, std::optional<llvm::Regex> pattern = std::nullopt) {
     ...
     ...
     so_addr.Dump(&strm, exe_scope, Address::DumpStyleResolvedDescription,
               Address::DumpStyleInvalid, UINT32_MAX, false, std::move(pattern));
     ...
     ...
     if (verbose) {
    strm.EOL();
    so_addr.Dump(&strm, exe_scope, Address::DumpStyleDetailedSymbolContext,
                 Address::DumpStyleInvalid, UINT32_MAX, all_ranges, std::move(pattern));
  }
     ...
}
```
The second `std::move(pattern)`, does not move the original pattern value but some garbage value. The only way is to make a copy of the original pattern and then use it with `std::move(copy_pattern)` every time (which doesn't look okay :-/).

This is the example code and output if I couldn't explain it properly above:
CODE:
```
static void test_func3(Stream &strm, const std::optional<llvm::Regex> pattern = std::nullopt) {
  if (pattern.has_value() && pattern.value().match("main")) {
        strm.Printf("Pattern matches 'main' inside test_func3\n");
    }
}

static uint32_t LookupSymbolInModule(CommandInterpreter &interpreter,
                                     Stream &strm, Module *module,
                                     const char *name, bool name_is_regex,
                                     bool verbose, bool all_ranges) {
  // test_func1(strm, std::make_optional<llvm::Regex>(name));
  std::optional<llvm::Regex> regex2 = std::make_optional<llvm::Regex>(name);
  if (regex2.has_value()) {
    strm.Printf("Pattern has a value\n");
    if(regex2.value().match("main")) {
      strm.Printf("Pattern matches main\n");
    }
    else{
      strm.Printf("Pattern doesn't match main\n");
    }
  } else {
    strm.Printf("Pattern doesn't have a value\n");
  }
  test_func3(strm, std::move(regex2));
  if (regex2.has_value()) {
    strm.Printf("Pattern has a value\n");
    if(regex2.value().match("main")) {
      strm.Printf("Pattern matches main\n");
    }
    else{
      strm.Printf("Pattern doesn't match main\n");
    }
  } else {
    strm.Printf("Pattern doesn't have a value\n");
  }  ...
  ...
}
```
OUTPUT:
```
talha at talha-ROG-Strix-G513QE-G513QE:~/Desktop/LLDB/build/bin$ ./lldb a.out 
(lldb) target create "a.out"
Current executable set to '/home/talha/Desktop/LLDB/build/bin/a.out' (x86_64).
(lldb) image lookup -r -s main
Pattern has a value
Pattern matches main
Pattern matches 'main' inside test_func3
Pattern has a value
Pattern doesn't match main
...
...
```
So after `std::move(pattern)`, `pattern != "main"` within the same function 

Similarly passing `std::make_optional<llvm::Regex>(name)` to func3 also causes problem problem:
i.e., `test_func3(strm, std::make_optional<llvm::Regex>(name));`

When we directly pass `std::make_optional<llvm::Regex>(name)`, it creates a temporary `std::optional<llvm::Regex>` object that is passed directly to `test_func3`. This temporary object can be moved without requiring an explicit `std::move` because I guess temporaries are considered rvalues, and `std::optional` has appropriate move constructors to handle them.
But storing the result of `std::make_optional<llvm::Regex>(name)` in a variable and then passing it without using `std::move`, if the variable is passed as an argument to the function, it attempts to perform a copy unless we use `std::move`. So, initially we can pass it directly to the next dumping function but further passing it will require `std::move` and the same problem comes back. 

Also 1 last thing for clarification, as discussed later i.e., 

> I think we may be able to reduce the boilerplate by putting the prefix/suffix/pattern in a struct, and passing that as an optional assuming we can figure out the move issue with regex. But this is for a future change if at all.

The struct will be optional and not the member variables inside it as we can check them in the very first function right? 
So the struct will look like:
```
struct information {
pattern, prefix, sufix
}
```
And information will be passed as optional argument rather than
```
struct information {
optional pattern = EMPTY, optional prefix= EMPTY, optional sufix = EMPTY
}
```

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