[Lldb-commits] [PATCH] D145574: [lldb] Read register fields from target XML

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.


================
Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4053
+      // appears once, so we don't have to handle that here.
+      if (attr_name == "name") {
+        LLDB_LOGF(log,
----------------
JDevlieghere wrote:
> Do you think that using a `StringSwitch` could improve readability? 
Unless I have the wrong end of the stick, StringSwitch is usually about returning a single value. Here we'd return a lambda for processing the attribute value.

That would look like:
```
      std::function<void(llvm::StringRef)> processor =
          llvm::StringSwitch<std::function<void(llvm::StringRef)>>(attr_name)
              .Case("name",
                    [&name, log](llvm::StringRef value) {
                      LLDB_LOGF(log,
                                "ProcessGDBRemote::ParseFlags Found field node "
                                "name \"%s\"",
                                value.data());
                      name = value;
                    })
              .Case("start",
                    [&start, log, max_start_bit](llvm::StringRef value) {
                      unsigned parsed_start = 0;
                      if (llvm::to_integer(value, parsed_start)) {
                        if (parsed_start > max_start_bit) {
                          LLDB_LOGF(log,
                                    "ProcessGDBRemote::ParseFlags Invalid "
                                    "start %u in field node, "
                                    "cannot be > %u",
                                    parsed_start, max_start_bit);
                        } else
                          start = parsed_start;
                      } else {
                        LLDB_LOGF(log,
                                  "ProcessGDBRemote::ParseFlags Invalid start "
                                  "\"%s\" in field node",
                                  value.data());
                      }
                    });

       processor(value);
```

Which seems about the same to me assuming one is comfortable reading lambdas, but tell me what you think. The extra indents seem to help more than the StringSwitch itself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145574