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

[Chris Lattner]

On Apr 1, 2010, at 6:48 PM, Sean Callanan wrote:

> We talked about this a little off-list, and decided to have the disassembler return an error code in all cases, but only print a debug message on NDEBUG builds.  The idea was to use the DEBUG macro and debugs() stream.
> 
> The attached patch implements these updates.  Please let me know what you think.

The patch generally looks good to me, remind me again why X86DisassemblerDecoder.c has to be a C file though?

-    translateInstruction(instr, internalInstr);
-    return true;
+    if (translateInstruction(instr, internalInstr))
+      return false;
+    else
+      return true;

This can just be: return !translateInstruction(...

However, it looks like translateInstruction can just return a bool instead of 0/-1.  If you do this, please use true for error to follow other llvm precedent, and allowing the ! to go away.  Likewise for translateRM etc.


+  if (!(insn.eaBase != EA_BASE_sib && insn.eaBase != EA_BASE_sib64)) {

Please apply demorgan's law to simplify the condition.


+  if (!(stackPos < 8)) {

Please simplify.

+#ifndef NDEBUG
+#define debug(s) do { x86DisassemblerDebug(__FILE__, __LINE__, s); } while (0);
+#else
+#define debug(s) do { } while (0);
+#endif

The typical do/while(0) idiom doesn't include the trailing ; in the macro.

Otherwise, looks fine, please commit with changes,

-Chris



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100401/6c03589a/attachment.html>