sammccall added inline comments.
================
Comment at: clang/lib/Format/ContinuationIndenter.cpp:289
Current.closesBlockOrBlockTypeList(Style)))
{
+ DEBUG_FORMAT_TRACE_TOKEN(Current, "!canBreak");
return false;
----------------
annotating all exit paths from this function and `mustBreak` seems much more
verbose and fragile than wrapping the function (making this version private)
and adding the annotations in the wrapper.
================
Comment at: clang/lib/Format/FormatDebug.h:22
+
+#ifndef NDEBUG
+
----------------
it looks like you're not providing a dummy definition in NDEBUG mode - does
this still build in that config?
================
Comment at: clang/lib/Format/FormatDebug.h:28
+
+#define DEBUG_FORMAT_TRACE_TOKEN(Tok, Debug)
\
+ if (internal::DebugTraceToken().match((Tok).TokenText)) {
\
----------------
Comment with an example?
(In particular, unclear what "Debug" means)
================
Comment at: clang/lib/Format/FormatDebug.h:32
+ << (Tok).TokenText << ": " << Debug << "\n";
\
+ } else {
\
+ }
----------------
why empty else?
================
Comment at: clang/lib/Format/TokenAnnotator.cpp:3258
}
+ DEBUG_FORMAT_TRACE_TOKEN(*Current, (Current->MustBreakBefore ? "" : "!")
+ << "MustBreakBefore");
----------------
move this out of the if() and remove the one on the other branch?
================
Comment at: clang/lib/Format/TokenAnnotator.cpp:3258
}
+ DEBUG_FORMAT_TRACE_TOKEN(*Current, (Current->MustBreakBefore ? "" : "!")
+ << "MustBreakBefore");
----------------
sammccall wrote:
> move this out of the if() and remove the one on the other branch?
this meaning of `<<` in `(...) << "MustBreakBefore` is confusing.
consider `<< (...) << "MustBreakBefore"` or `dbgs() << (...) <<
"MustBreakBefore"` or a twine or formatv or really anything that isn't a shift
:-)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145244/new/
https://reviews.llvm.org/D145244
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
