HazardyKnusperkeks marked 3 inline comments as done. HazardyKnusperkeks added inline comments.
================ Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:215 + const auto &NextLine = *I[1]; + const auto *PreviousLine = I != AnnotatedLines.begin() ? I[-1] : nullptr; + if (NextLine.Type == LT_Invalid || NextLine.First->MustBreakBefore) ---------------- owenpan wrote: > HazardyKnusperkeks wrote: > > owenpan wrote: > > > I would move this line to just before handling empty record blocks below. > > I'd rather keep the definitions close together. > If it were just a simple initialization, it wouldn't matter much. However, it > would be a bit wasteful as `PreviousLine` always gets computed here even if > the function may return before the pointer would get chance to be used. If > you really want to keep all related definitions together, wouldn't you want > to move lines 214-215 up to right after line 211? In a release build I'm betting that the compiler is smart enough to never calculate `PreviousLine` and that performance doesn't really matter was shown in D116318. But yeah moving it up to `TheLine` is consistent and will do. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115060/new/ https://reviews.llvm.org/D115060 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
