[Lldb-commits] [PATCH] D30402: Modernize Enable/DisableLogChannel interface a bit

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.


================
Comment at: include/lldb/Interpreter/Args.h:196-197
+  llvm::ArrayRef<const char *> GetArgumentArrayRef() const {
+    return {static_cast<const char *const *>(m_argv.data()),
+            m_argv.size() - 1};
+  }
----------------
can this be written `return makeArrayRef(m_argv).drop_back();`?


================
Comment at: source/API/SBDebugger.cpp:1129
+    ++len;
+  return {categories, len};
+}
----------------
`makeArrayRef(categories, len)` again seems more readable.


================
Comment at: source/Core/Log.cpp:74-80
+    auto cat = llvm::find_if(
+        entry.second.channel.categories,
+        [&](const Log::Category &c) { return c.name.equals_lower(category); });
     if (cat != entry.second.channel.categories.end()) {
       flags |= cat->flag;
       continue;
     }
----------------
How about 

```
bool exists = llvm::any(entry.second.channel.categories,
        [&](const Log::Category &c) { return c.name.equals_lower(category); }));
if (exists) {
  ...
}
```


================
Comment at: unittests/Interpreter/TestArgs.cpp:174
+  Args args("foo bar");
+  llvm::ArrayRef<const char *> ref = args.GetArgumentArrayRef();
+  ASSERT_EQ(2u, ref.size());
----------------
I'd probably use `auto` here since the function indicates that it's returning an `ArrayRef`.  I don't feel strongly though.


https://reviews.llvm.org/D30402