Completes the set.
Details
Diff Detail
Event Timeline
include/llvm/Support/Format.h | ||
---|---|---|
138 | Why don't you use Justification as a type? I think you don't need to save a few bits here. | |
lib/Support/raw_ostream.cpp | ||
333–342 | Using an enum value as a bitmask looks too tricky. It is probably better to write in three-way branches like this. switch (FS.Justify) { case JustifyRight: this->indent(FS.Width - Len); this->operator<<(FS.Str); break; case JustifyCenter: { int PadAmount = (FS.Width - Len) / 2; this->indent(PadAmount); this->operator<<(FS.Str); this->indent(Difference - PadAmount * 2); } case JustifyLeft: this->operator<<(FS.Str); this->indent(FS.Width - Len); break; } |
lib/Support/raw_ostream.cpp | ||
---|---|---|
344 | nit; since the scope of this variable is very small, I wouldn't add const as it is obvious. | |
347 | PadAmount + (Difference - (PadAmount * 2)) is the same as PadAmount + Difference - PadAmount * 2 and thus Difference - PadAmount ? | |
348 | I believe we usually write this way. break; } |
lib/Support/raw_ostream.cpp | ||
---|---|---|
347 | Isn't this the same as Difference - PadAmount as I wrote above? |
It looks good to me, but I'll LGTM tomorrow to give other reviewers a chance to take a look.
Why don't you use Justification as a type? I think you don't need to save a few bits here.