JDevlieghere accepted this revision.
JDevlieghere 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,
----------------
DavidSpickett wrote:
> 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.
Fair enough, and it's probably not worth creating an enum for this (which is
what I originally had in mind). I think the code is fine as it is.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145574/new/
https://reviews.llvm.org/D145574
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
