labath added a comment. Hopefully last round of cosmetic fixes, and then this should be good.
================ Comment at: lldb/include/lldb/Target/MemoryRegionInfo.h:15 #include "lldb/Utility/RangeMap.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/FormatProviders.h" ---------------- I guess this is not used anymore.. ================ Comment at: lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py:737 "name", - "error"]) + "error"], "Unexpected key \"%s\"" % key) self.assertIsNotNone(val) ---------------- I guess you could change this to `self.assertIn(needle, haystack)` ================ Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1309-1313 + llvm::handleAllErrors(Info.takeError(), + [&Result](const llvm::StringError &e) { + Result.SetErrorToGenericError(); + Result.SetErrorString(e.getMessage()); + }); ---------------- Status has an llvm::Error constructor. Some variation on `Result = Info.takeError()` should work. ================ Comment at: lldb/source/Plugins/Process/Utility/LinuxProcMaps.cpp:196 + callback(*region); + } +} ---------------- [[ http://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements | no braces for simple statements]] ================ Comment at: lldb/source/Plugins/Process/minidump/MinidumpParser.cpp:270-272 + llvm::handleAllErrors( + region.takeError(), + [](const llvm::StringError &e) {}); ---------------- `llvm::consumeError(region.takeError())`, though maybe it would be better to actually log it (LLDB_LOG_ERROR) ================ Comment at: lldb/test/API/linux/aarch64/mte_memory_region/TestAArch64LinuxMTEMemoryRegion.py:31 + # 47 is our magic status meaning MTE isn't available + if "exited with status = 47" in self.res.GetOutput(): + self.skipTest("MTE must be available in toolchain and on target") ---------------- `if self.process().GetState() == lldb.eStateExited and self.process().GetExitStatus() == 47` would be nicer ================ Comment at: lldb/test/API/linux/aarch64/mte_memory_region/TestAArch64LinuxMTEMemoryRegion.py:34-38 + lldbutil.run_break_set_by_file_and_line(self, "main.c", + line_number('main.c', '// Set break point at this line.'), + num_expected_locations=1) + + self.runCmd("run", RUN_SUCCEEDED) ---------------- I think you could just run this once, and then choose to skip the test or not depending on whether the process exited, or hit a breakpoint. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87442/new/ https://reviews.llvm.org/D87442 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits