[PATCH] D60376: [llvm-objdump] Align instructions to a tab stop

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked an inline comment as done.
MaskRay added inline comments.


================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:622
+      unsigned Column = FOS.getColumn();
+      FOS.indent(Column < TabStop - 1 ? TabStop - 1 - Column : 7 - Column % 8);
+
----------------
jhenderson wrote:
> MaskRay wrote:
> > jhenderson wrote:
> > > I can never remember the precedence of `%`, so would you mind adding some brackets to the LHS.
> > > 
> > > `7 - Column` seems a bit weird given that `Column` is always going to be bigger than that. Could you rewrite this bit so that it doesn't rely on underflowing?
> > Multiplication, division and remainder have the same precedence. Just as `1 - 2 / 3` doesn't need parentheses, the remainder expression doesn't need parentheses.
> > 
> > `7 - Column % 8` computes to the range 0~7, there is no underflow.
> Oh, right, that's me misremembering things. Even more of a reason to add brackets around `Column % 8` then. They aren't needed for correctness, but are for readability.
I don't agree. For me, the redundant pair of parentheses harms readability.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60376