[Lldb-commits] [PATCH] D78825: [lldb/Driver] Exit with a non-zero exit code in batch mode when stopping because of an error.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

(I made this comment yesterday, but it seems I didn't click send)



================
Comment at: lldb/source/API/SBDebugger.cpp:1205-1209
                                  options.ref());
     num_errors = interp.GetNumErrors();
     quit_requested = interp.GetQuitRequested();
     stopped_for_crash = interp.GetStoppedForCrash();
+    stopped_for_error = interp.GetStoppedForError();
----------------
JDevlieghere wrote:
> labath wrote:
> > What's the relationship between `num_errors` and `stopped_for_error`? Could we infer that we stopped because of an error if `num_errors>0` ?
> I considered that, `m_num_errors` is incremented unconditionally, while stopped_for_error is only set when `eHandleCommandFlagStopOnError` is set. If you know that "stop on error" is set you could infer it based on the value being non-zero, but to me that sounds a bit fragile, and given that we already have `stopped_for_crash` I preferred the symmetry. I'm not married to this approach so if you feel strongly about it I'm happy to change it. 
Yea, needing to remember what you set as the value for "stop on error" does make this fairly finicky. OTOH, one could argue that errors shouldn't be counted as real errors (and reported here) unless stop-on-error is set. That would make checking for error-stops easier.

I don't feel strongly about any of that, but I can't say I am thrilled by the prospect of introducing another overload. So, if we do that, I'd like to ensure some thought goes into it.

Greg's idea of making a struct for the return values sounds like a good one -- having so many by-ref results is not very nice (I don't like having even one tbh). My question there would be should we have separate bool flags for the "stop reasons". It seems like these conditions should be mutually exclusive, so it may be better to have an enum to indicate a reason for stopping (+ an accessor for the error count, I guess -- though I am not exactly sure how useful it is to know the exact number of errors without being able to access them in any way).


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78825/new/

https://reviews.llvm.org/D78825