[PATCH] D145244: [clang-format] Add ability to trace tokens.

Manuel Klimek via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 6 02:50:15 PST 2023


klimek added a comment.

Answering the fundamental design question first.



================
Comment at: clang/lib/Format/ContinuationIndenter.cpp:289
                                    Current.closesBlockOrBlockTypeList(Style))) {
+    DEBUG_FORMAT_TRACE_TOKEN(Current, "!canBreak");
     return false;
----------------
sammccall wrote:
> 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.
How do we get the exact condition that triggered? The main trick here is the __FILE__:__LINE__ in the debug output.


================
Comment at: clang/lib/Format/FormatDebug.h:22
+
+#ifndef NDEBUG
+
----------------
sammccall wrote:
> it looks like you're not providing a dummy definition in NDEBUG mode - does this still build in that config?
All use is within LLVM_DEBUG().


================
Comment at: clang/lib/Format/FormatDebug.h:32
+                 << (Tok).TokenText << ": " << Debug << "\n";                  \
+  } else {                                                                     \
+  }
----------------
sammccall wrote:
> why empty else?
The idea was dangling else protection, but I guess we have dangling else warnings, so this is not necessary?


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:3258
       }
+      DEBUG_FORMAT_TRACE_TOKEN(*Current, (Current->MustBreakBefore ? "" : "!")
+                                             << "MustBreakBefore");
----------------
sammccall wrote:
> 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 :-)
The idea was to see which branch of the if is taken here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145244



More information about the cfe-commits mailing list