[PATCH] D145244: [clang-format] Add ability to trace tokens.
sammccall added a comment.
Oops, sorry for the noise.
I'd forgotten that we're really interested in *why* we reach the conclusion,
not the conclusion itself - and that file/line are our proxy for "why" as
writing/maintaining descriptions is a burden.
Comment at: clang/lib/Format/ContinuationIndenter.cpp:289
Current.closesBlockOrBlockTypeList(Style)))
{
+DEBUG_FORMAT_TRACE_TOKEN(Current, "!canBreak");
return false;
klimek wrote:
> 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.
Doh. There *is* an a way in recent clang/gcc/msvc, though I suspect you don't
want to go down this path...
```
template
struct LogReturn {
T Value;
#ifdef NDEBUG
LogReturn(T Value) : Value(Value) {}
#else
const char *File;
int Line;
// default args are evaluated at callsite
LogReturn(T Value, const char *File=__builtin_FILE(), int
Line=__builtin_LINE()) : Value(Value), File(File), Line(Line) {
dbgs() << ...;
}
#endif
}
LogReturn priv() { return true; }
bool pub() { return priv().Value; }
```
Dunno how to get the token without resorting to TLS though. More realistically
you could define `trace(bool, Token&, const char*File=..., etc)` and `return
trace(true, Tok);`
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
[PATCH] D145244: [clang-format] Add ability to trace tokens.
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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D145244: [clang-format] Add ability to trace tokens.
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
[PATCH] D145244: [clang-format] Add ability to trace tokens.
klimek updated this revision to Diff 502123.
klimek added a comment.
Remove superfluous include.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145244/new/
https://reviews.llvm.org/D145244
Files:
clang/lib/Format/CMakeLists.txt
clang/lib/Format/ContinuationIndenter.cpp
clang/lib/Format/FormatDebug.cpp
clang/lib/Format/FormatDebug.h
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/UnwrappedLineFormatter.cpp
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -7,6 +7,7 @@
//===--===//
#include "UnwrappedLineFormatter.h"
+#include "FormatDebug.h"
#include "FormatToken.h"
#include "NamespaceEndCommentsFixer.h"
#include "WhitespaceManager.h"
@@ -1252,17 +1253,43 @@
/// penalty of \p Penalty. Insert a line break if \p NewLine is \c true.
void addNextStateToQueue(unsigned Penalty, StateNode *PreviousNode,
bool NewLine, unsigned *Count, QueueType *Queue) {
-if (NewLine && !Indenter->canBreak(PreviousNode->State))
+bool MustBreak = false;
+bool MustNotBreak = false;
+LLVM_DEBUG({
+ if (internal::DebugForceMustBreak().match(
+ PreviousNode->State.NextToken->TokenText)) {
+MustBreak = true;
+ }
+ if (internal::DebugForceMustNotBreak().match(
+ PreviousNode->State.NextToken->TokenText)) {
+MustNotBreak = true;
+ }
+});
+
+if (NewLine && (!Indenter->canBreak(PreviousNode->State) || MustNotBreak) &&
+!MustBreak) {
+ DEBUG_FORMAT_TRACE_TOKEN(*PreviousNode->State.NextToken,
+ "Cannot break before");
return;
-if (!NewLine && Indenter->mustBreak(PreviousNode->State))
+}
+if (!NewLine && (Indenter->mustBreak(PreviousNode->State) || MustBreak) &&
+!MustNotBreak) {
+ DEBUG_FORMAT_TRACE_TOKEN(*PreviousNode->State.NextToken,
+ "Must break before");
return;
+}
StateNode *Node = new (Allocator.Allocate())
StateNode(PreviousNode->State, NewLine, PreviousNode);
if (!formatChildren(Node->State, NewLine, /*DryRun=*/true, Penalty))
return;
-Penalty += Indenter->addTokenToState(Node->State, NewLine, true);
+unsigned AddPenalty = Indenter->addTokenToState(Node->State, NewLine, true);
+DEBUG_FORMAT_TRACE_TOKEN(*PreviousNode->State.NextToken,
+ "Penalty for " << (!NewLine ? "not " : "")
+<< "breaking before token is "
+<< AddPenalty);
+Penalty += AddPenalty;
Queue->push(QueueItem(OrderedPenalty(Penalty, *Count), Node));
++(*Count);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -13,6 +13,8 @@
//===--===//
#include "TokenAnnotator.h"
+#include "ContinuationIndenter.h"
+#include "FormatDebug.h"
#include "FormatToken.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TokenKinds.h"
@@ -3244,6 +3246,7 @@
const auto = Prev->Children;
if (!Children.empty() && Children.back()->Last->is(TT_LineComment)) {
+ DEBUG_FORMAT_TRACE_TOKEN(*Current, "MustBreakBefore");
Current->MustBreakBefore = true;
} else {
Current->MustBreakBefore =
@@ -3252,6 +3255,8 @@
Current->is(TT_FunctionDeclarationName)) {
Current->MustBreakBefore = mustBreakForReturnType(Line);
}
+ DEBUG_FORMAT_TRACE_TOKEN(*Current, (Current->MustBreakBefore ? "" : "!")
+ << "MustBreakBefore");
}
Current->CanBreakBefore =
Index: clang/lib/Format/FormatDebug.h
===
--- /dev/null
+++ clang/lib/Format/FormatDebug.h
@@ -0,0 +1,41 @@
+//===--- FormatDebug.h - Format C++ code *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+///
+/// \file
+/// This file declares utilities used in clang-format in debug mode.
+///
+//===--===//
+#ifndef LLVM_CLANG_LIB_FORMAT_FORMATDEBUG_H
+#define LLVM_CLANG_LIB_FORMAT_FORMATDEBUG_H
+
+#include "llvm/Support/Regex.h"
+
+namespace clang {
+namespace format {
+namespace internal {
+
+#ifndef NDEBUG
+
[PATCH] D145244: [clang-format] Add ability to trace tokens.
klimek created this revision.
klimek added a reviewer: sammccall.
Herald added a project: All.
klimek requested review of this revision.
Herald added a project: clang.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D145244
Files:
clang/lib/Format/CMakeLists.txt
clang/lib/Format/ContinuationIndenter.cpp
clang/lib/Format/FormatDebug.cpp
clang/lib/Format/FormatDebug.h
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/UnwrappedLineFormatter.cpp
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -7,6 +7,8 @@
//===--===//
#include "UnwrappedLineFormatter.h"
+#include "ContinuationIndenter.h"
+#include "FormatDebug.h"
#include "FormatToken.h"
#include "NamespaceEndCommentsFixer.h"
#include "WhitespaceManager.h"
@@ -1252,17 +1254,43 @@
/// penalty of \p Penalty. Insert a line break if \p NewLine is \c true.
void addNextStateToQueue(unsigned Penalty, StateNode *PreviousNode,
bool NewLine, unsigned *Count, QueueType *Queue) {
-if (NewLine && !Indenter->canBreak(PreviousNode->State))
+bool MustBreak = false;
+bool MustNotBreak = false;
+LLVM_DEBUG({
+ if (internal::DebugForceMustBreak().match(
+ PreviousNode->State.NextToken->TokenText)) {
+MustBreak = true;
+ }
+ if (internal::DebugForceMustNotBreak().match(
+ PreviousNode->State.NextToken->TokenText)) {
+MustNotBreak = true;
+ }
+});
+
+if (NewLine && (!Indenter->canBreak(PreviousNode->State) || MustNotBreak) &&
+!MustBreak) {
+ DEBUG_FORMAT_TRACE_TOKEN(*PreviousNode->State.NextToken,
+ "Cannot break before");
return;
-if (!NewLine && Indenter->mustBreak(PreviousNode->State))
+}
+if (!NewLine && (Indenter->mustBreak(PreviousNode->State) || MustBreak) &&
+!MustNotBreak) {
+ DEBUG_FORMAT_TRACE_TOKEN(*PreviousNode->State.NextToken,
+ "Must break before");
return;
+}
StateNode *Node = new (Allocator.Allocate())
StateNode(PreviousNode->State, NewLine, PreviousNode);
if (!formatChildren(Node->State, NewLine, /*DryRun=*/true, Penalty))
return;
-Penalty += Indenter->addTokenToState(Node->State, NewLine, true);
+unsigned AddPenalty = Indenter->addTokenToState(Node->State, NewLine, true);
+DEBUG_FORMAT_TRACE_TOKEN(*PreviousNode->State.NextToken,
+ "Penalty for " << (!NewLine ? "not " : "")
+<< "breaking before token is "
+<< AddPenalty);
+Penalty += AddPenalty;
Queue->push(QueueItem(OrderedPenalty(Penalty, *Count), Node));
++(*Count);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -13,6 +13,8 @@
//===--===//
#include "TokenAnnotator.h"
+#include "ContinuationIndenter.h"
+#include "FormatDebug.h"
#include "FormatToken.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TokenKinds.h"
@@ -3244,6 +3246,7 @@
const auto = Prev->Children;
if (!Children.empty() && Children.back()->Last->is(TT_LineComment)) {
+ DEBUG_FORMAT_TRACE_TOKEN(*Current, "MustBreakBefore");
Current->MustBreakBefore = true;
} else {
Current->MustBreakBefore =
@@ -3252,6 +3255,8 @@
Current->is(TT_FunctionDeclarationName)) {
Current->MustBreakBefore = mustBreakForReturnType(Line);
}
+ DEBUG_FORMAT_TRACE_TOKEN(*Current, (Current->MustBreakBefore ? "" : "!")
+ << "MustBreakBefore");
}
Current->CanBreakBefore =
Index: clang/lib/Format/FormatDebug.h
===
--- /dev/null
+++ clang/lib/Format/FormatDebug.h
@@ -0,0 +1,41 @@
+//===--- FormatDebug.h - Format C++ code *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+///
+/// \file
+/// This file declares utilities used in clang-format in debug mode.
+///
+//===--===//
+#ifndef LLVM_CLANG_LIB_FORMAT_FORMATDEBUG_H
+#define LLVM_CLANG_LIB_FORMAT_FORMATDEBUG_H
+
+#include "llvm/Support/Regex.h"
+
+namespace clang {
+namespace format {
