owenpan 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)
----------------
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?


================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:268-289
+        [this, B = AnnotatedLines.begin(), &NextLine,
+         TheLine](SmallVectorImpl<AnnotatedLine *>::const_iterator I) {
           if (Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All)
             return true;
           if (Style.AllowShortFunctionsOnASingleLine >=
                   FormatStyle::SFS_Empty &&
+              NextLine.First->is(tok::r_brace))
----------------
HazardyKnusperkeks wrote:
> owenpan wrote:
> > I'd either leave this lambda alone or further simplify it as suggested here.
> I'm no fan of capturing everything, but some things I've applied.
`[&]` doesn't capture everything. It only captures the variables that are used 
in the lambda body. That being said, I'm okay with explicitly capturing each 
one.


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

Reply via email to