[Lldb-commits] [PATCH] D61847: [lldb] modernize codebase with modernize-use-nullptr

Konrad Wilhelm Kleine via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Mon May 13 02:25:30 PDT 2019


kwk added inline comments.


================
Comment at: lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp:76
 static constexpr PropertyDefinition g_properties[] = {
-    {"load-kexts", OptionValue::eTypeBoolean, true, true, NULL, {},
+    {"load-kexts",
+     OptionValue::eTypeBoolean,
----------------
teemperor wrote:
> Can you revert the auto-formatting here?
@teemperor I sure could or if you want I can sourround this section with ` // clang-format off` and ` // clang-format on` to prevent it from being ever formatted again. 


================
Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:114
 static constexpr PropertyDefinition g_properties[] = {
-    {"packet-timeout", OptionValue::eTypeUInt64, true, 5
+    {"packet-timeout",
+     OptionValue::eTypeUInt64,
----------------
teemperor wrote:
> Can you revert the auto-formatting here (and in the next option definition)?
@teemperor sure but same question: shall I use ` // clang-format off` and ` // clang-format on` here for the entire section? 


================
Comment at: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp:603
 public:
-
   CommandObjectProcessMinidumpDump(CommandInterpreter &interpreter)
+      : CommandObjectParsed(interpreter, "process plugin dump",
----------------
teemperor wrote:
> Could you also revert the unrelated whitespace changes here? Phabricator doesn't really display the whitespace changes well, but this patch effectively reformats this whole INIT_BOOL chain (The reformatting itself is fine, but it should be it's own commit).
@teemperor sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61847





More information about the lldb-commits mailing list