[llvm-commits] Error-handling fixes for the x86 disassembler

[Daniel Dunbar]

Hi Sean,

A couple comments:
 (1) Any new error handling code inside lib/ should not use something
like fprintf to stderr, rather it should use a clean API to
communicate errors up to clients.
 (2) Various nit picks like, 'if(' -> 'if (', don't implement
functionality with macros unless absolutely necessary, use 'bool'
instead of int when result is a bool, and everything Eric said.

For something like the disassembler, an approach we use in Clang and
other places is that the parsing functions should *always* recover and
not worry about communicating the error status up to their caller.
Instead they should just emit the diagnostic using a clean client API,
and the client is responsible for knowing that once an error is
encountered, it should limit its expectations on the result.

For tools which should never fail, and can always recover (the
disassembler should fit this), this makes the code cleaner because it
doesn't have to worry about unwinding on errors, instead it just
recovers by returning a result which is structurally sound, but the
client knows it can't rely on it to be valid because an error was
generated.

 - Daniel

On Wed, Mar 31, 2010 at 3:42 PM, Sean Callanan <scallanan at apple.com> wrote:
> The attached patch fixes error handling for the x86 disassembler, eliminating llvm_unreachable() and assert() in favor of using the legitimate error-handling mechanism.  The rationale is to allow clients of libEnhancedDisassembly to handle errors themselves, rather than forcing them to crash.
>
> The difference is that now assert() and llvm_unreachable() will never be called (even before, this was extremely unlikely), and the proper error-handling mechanism is used instead.  Further diagnostic information is printed to stderr.
>
> Please let me know what you think.
>
> Sean
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>