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

Reply via email to