labath added inline comments.

================
Comment at: tools/lldb-test/lldb-test.cpp:260
+    if (!RE.IsValid()) {
+      errs() << "ERROR: `" << Name << "` is not a valid regular expression.\n";
+      return;
----------------
JDevlieghere wrote:
> JDevlieghere wrote:
> > You can use `WithColor::error()` from Support :-) 
> I probably should've noticed this earlier, but since we return after all 
> these errors maybe I'd be nice to have the function return an error? Doesn't 
> have to be in this diff though. 
I've handled this a bit differently. Since this will be the same regex for each 
module we dump, I've done the validation up-front (next to the other checks), 
and changed this into an assertion.


================
Comment at: tools/lldb-test/lldb-test.cpp:463
 
   DebuggerLifetime->Terminate();
   return 0;
----------------
JDevlieghere wrote:
> If we can wrap this in a RAII object we could have this tool return a 
> non-zero exit code in case of an error. 
Done. I've made the individual functions return an `int`, and had `main` 
forward that. The reason I am not having the functions return an `Error` or 
like is that the functions generally do the same thing over several inputs, and 
the errors that happen when processing one of the inputs should be printed next 
to that input and not deferred until the end. So, the functions just accumulate 
a flag saying whether they encountered any errors and then return that.


https://reviews.llvm.org/D46318



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to