lawrence_danna added inline comments.
================ Comment at: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:58-61 + auto utf8 = str.AsUTF8(); + if (!utf8) + return utf8.takeError(); + return utf8.get(); ---------------- labath wrote: > Btw, what are the reasons that this can fail? The string object was created > two lines above, so we definitely know it's a string. If the call cannot fail > in this particular circumstance, then a one can express that (and make the > code simpler) with `return cantFail(str.AsUTF8())`. It is possible! ``` chr(0xDFFF).encode('utf8') ``` ================ Comment at: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:864-891 + auto function = As<PythonCallable>(globals.GetItem("get_arg_info")); + if (!function) + return function.takeError(); + get_arg_info = std::move(function.get()); + } + + Expected<PythonObject> pyarginfo = get_arg_info.Call(*this); ---------------- labath wrote: > Likewise, should most of these `takeErrors` be `cantFail` too ? Yea, i guess given that we control the script there's really no reasonable way these can fail if the first check doesn't. ================ Comment at: lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp:643-647 + auto arginfo = lambda.GetArgInfo(); + ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded()); + EXPECT_EQ(arginfo.get().count, 1); + EXPECT_EQ(arginfo.get().has_varargs, false); + EXPECT_EQ(arginfo.get().is_bound_method, false); ---------------- labath wrote: > If you implemented `operator==` and `operator<<(raw_ostream&)` for the > ArgInfo class, you could write this as > `EXPECT_THAT_EXPECTED(lambda.GetArgInfo(), llvm::HasValue(ArgInfo(1, false, > false))` eh, do i have to? I actually like having them each on their own line. It makes the patches nicer when members are added or removed, and i'd like to remove all the members except max_positional_args. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68995/new/ https://reviews.llvm.org/D68995 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits