BTW, I'm interested to know if you have some analysis which leads you to think
that propagating unchecked errors actually is a big problem in lldb, or are you
just doing this to remove a spelling collision? I see a lot of bugs for lldb
come by, but I really haven't seen many that this sort of forced checking would
have fixed.
Jim
> On May 1, 2017, at 12:36 PM, Jim Ingham <jing...@apple.com> wrote:
>
>>
>> On May 1, 2017, at 11:48 AM, Zachary Turner <ztur...@google.com> wrote:
>>
>>
>>
>> On Mon, May 1, 2017 at 11:28 AM Jim Ingham <jing...@apple.com> wrote:
>> I'm mostly but not entirely tongue in cheek wondering why we aren't calling
>> llvm::Error llvm::LLVMError, as the lldb error class much preceded it, but
>> that's a minor point.
>> FWIW I think the naming chosen by LLVM is correct. It's intended to be a
>> generic utility class, extensible enough to be used by anything that links
>> against LLVM. As such, calling it LLVMError kind of gives off the false
>> impression that it should only be used by errors that originate from LLVM,
>> when in fact it's much more general purpose.
>>
>>
>> If it is actually causing confusion (I haven't experienced such yet) I don't
>> mind typing some extra letters.
>> I think that's in part because llvm::Error isn't very prevalent inside of
>> LLDB (yet).
>>
>>
>> As we've discussed several times in the past, we often use errors for
>> informational purposes (for instance in the ValueObject system) with no
>> programmatic requirement they be checked. So the llvm::Error class is not a
>> drop-in replacement for our uses of lldb_private::Error in subset of its
>> uses. More generally, the environment the debugger lives in is often pretty
>> dirty, bad connections to devices, uncertain debug information, arguments
>> with clang about what types mean, weird user input, etc. But the job of the
>> debugger is to keep going as well/long as it can in the face of this. For
>> something like a compiler, if some operation goes bad that whole execution
>> is likely rendered moot, and so bagging out early is the right thing to do.
>> For lldb, if the debug info for a frame is all horked up, users can still
>> resort to memory reading and casts, or some other workaround, provided the
>> debugger stays alive. This makes me a little leery of adopting an
>> infrastructure whose default action is to abort on mishandling.
>> Just re-iterating from previous discussions, but it only does that in debug
>> mode. When you have a release build, it will happily continue on without
>> aborting. The point of all this is that you catch unhandled errors
>> immediately the first time you run the test suite.
>
> Yup, we do that for assertions. But asserting isn't appropriate even in the
> testsuite for cases where we don't require the errors be checked.
>
>>
>> Even if you have a bad connection, uncertain debug information, etc you
>> still have to propagate that up the callstack some number of levels until
>> someone knows what to do. All this class does is make sure (when in debug
>> mode) that you're doing that instead of silently ignoring some condition.
>>
>> That said, it certainly seems plausible that we could come up with some kind
>> of abstraction for informational status messages. With that in mind, I'll
>> change my original renaming proposal from LLDBError to Status. This way we
>> will have llvm::Error and lldb_private::Status.
>
> That seems better.
>
>>
>> In the future, perhaps we can discuss with Lang and the larger community
>> about whether such a class makes in LLVM as well. Maybe there's a way to
>> get both checked and unchecked errors into LLVM using a single consistent
>> interface. But at least then the person who generates the error is
>> responsible for deciding how important it is.
>>
>
> It's not "how important it is" but "does this error need to be dealt with
> programmatically proximate to the code that produces it." For instance, if
> an error makes it to the SB API level - something that is quite appropriate
> for the SBValues for instance, we wouldn't want to use an llvm::Error. After
> all forcing everybody to check this at the Python layer would be really
> annoying. I guess you could work around this by hand-checking off any error
> when you go from lldb_private -> SBError. But that seems like now you're
> just pretending to be doing something you aren't, which I don't think is
> helpful.
>
> Probably better as you say to make everything into lldb_private::Status
> behaving as it does now, to side-step the name collision, and then start with
> all the uses where the error doesn't propagate very far, and try converting
> those to use llvm::Error and working judiciously out from there. 'Course you
> can't change the SB API names, so there will always be a little twist there.
>
> Jim
>
>>
>>
>> BTW, I don't think the comment Lang cited had to do with replacing the
>> errors with some other error backend. It was more intended to handle a
>> problem that came up with gdb where we tried to multiplex all various system
>> error numbers into one single error. lldb errors have a flavor
>> (eErrorTypePosix, eErrorTypeWin32, etc) which allows you to use each native
>> error number by annotating it with the flavor.
>>
>> FWIW, using the llvm::Error model, the way this is handled is by doing
>> something like this:
>>
>> return make_error<WindowsError>(::GetLastError());
>>
>> return make_error<ErrnoError>(errno);
>>
>> but it is general enough to handle completely different categories of errors
>> as well, so you can "namespace" out your command interpreter errors, debug
>> info errors, etc.
>>
>> return make_error<CommandInterpreterError>("Incorrect command usage");
>>
>> return make_error<DWARFFormatError>("Invalid DIE specification");
>>
>> etc
_______________________________________________
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
