xiaobai added a comment. I like the idea of using something from llvm instead of rolling our own. The code changes look relatively simple and straightforward, so that's good.
================ Comment at: lldb/source/Commands/CommandObjectBreakpoint.cpp:685 RegularExpression regexp(m_options.m_func_regexp); - if (!regexp.IsValid()) { - char err_str[1024]; - regexp.GetErrorAsCString(err_str, sizeof(err_str)); + if (auto error = regexp.GetError()) { result.AppendErrorWithFormat( ---------------- labath wrote: > I wouldn't use `auto` in cases like these because `Optional<string>` is > definitely not among the things one would expect a function called `GetError` > to return. Though maybe that means the function should really return an > llvm::Error? If Pavel hadn't pointed this out, I would have thought that `error` was an `llvm::Error` here and not a an Optional. I think something should change here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66174/new/ https://reviews.llvm.org/D66174 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits