-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[llvm][mustache] Simplify debug logging #159193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-llvm-support Author: Paul Kirth (ilovepi) ChangesFull diff: https://github.com/llvm/llvm-project/pull/159193.diff 1 Files Affected:
diff --git a/llvm/lib/Support/Mustache.cpp b/llvm/lib/Support/Mustache.cpp
index e084e379312ef..6dab658ea535d 100644
--- a/llvm/lib/Support/Mustache.cpp
+++ b/llvm/lib/Support/Mustache.cpp
@@ -187,14 +187,14 @@ class ASTNode {
void render(const llvm::json::Value &Data, MustacheOutputStream &OS);
private:
- void renderLambdas(const llvm::json::Value &Contexts, MustacheOutputStream &OS,
- Lambda &L);
+ void renderLambdas(const llvm::json::Value &Contexts,
+ MustacheOutputStream &OS, Lambda &L);
void renderSectionLambdas(const llvm::json::Value &Contexts,
MustacheOutputStream &OS, SectionLambda &L);
- void renderPartial(const llvm::json::Value &Contexts, MustacheOutputStream &OS,
- ASTNode *Partial);
+ void renderPartial(const llvm::json::Value &Contexts,
+ MustacheOutputStream &OS, ASTNode *Partial);
void renderChild(const llvm::json::Value &Context, MustacheOutputStream &OS);
@@ -204,9 +204,11 @@ class ASTNode {
void renderText(MustacheOutputStream &OS);
void renderPartial(const json::Value &CurrentCtx, MustacheOutputStream &OS);
void renderVariable(const json::Value &CurrentCtx, MustacheOutputStream &OS);
- void renderUnescapeVariable(const json::Value &CurrentCtx, MustacheOutputStream &OS);
+ void renderUnescapeVariable(const json::Value &CurrentCtx,
+ MustacheOutputStream &OS);
void renderSection(const json::Value &CurrentCtx, MustacheOutputStream &OS);
- void renderInvertSection(const json::Value &CurrentCtx, MustacheOutputStream &OS);
+ void renderInvertSection(const json::Value &CurrentCtx,
+ MustacheOutputStream &OS);
MustacheContext &Ctx;
Type Ty;
@@ -330,6 +332,36 @@ struct Tag {
size_t StartPosition = StringRef::npos;
};
+static const char *tagKindToString(Tag::Kind K) {
+ switch (K) {
+ case Tag::Kind::None:
+ return "None";
+ case Tag::Kind::Normal:
+ return "Normal";
+ case Tag::Kind::Triple:
+ return "Triple";
+ }
+ llvm_unreachable("Unknown Tag::Kind");
+}
+
+static const char *jsonKindToString(json::Value::Kind K) {
+ switch (K) {
+ case json::Value::Kind::Null:
+ return "JSON_KIND_NULL";
+ case json::Value::Kind::Boolean:
+ return "JSON_KIND_BOOLEAN";
+ case json::Value::Kind::Number:
+ return "JSON_KIND_NUMBER";
+ case json::Value::Kind::String:
+ return "JSON_KIND_STRING";
+ case json::Value::Kind::Array:
+ return "JSON_KIND_ARRAY";
+ case json::Value::Kind::Object:
+ return "JSON_KIND_OBJECT";
+ }
+ llvm_unreachable("Unknown json::Value::Kind");
+}
+
static Tag findNextTag(StringRef Template, size_t StartPos, StringRef Open,
StringRef Close) {
const StringLiteral TripleOpen("{{{");
@@ -374,11 +406,10 @@ static Tag findNextTag(StringRef Template, size_t StartPos, StringRef Open,
static std::optional<std::pair<StringRef, StringRef>>
processTag(const Tag &T, SmallVectorImpl<Token> &Tokens) {
- LLVM_DEBUG(dbgs() << " Found tag: \"" << T.FullMatch << "\", Content: \""
- << T.Content << "\"\n");
+ LLVM_DEBUG(dbgs() << "[Tag] " << T.FullMatch << ", Content: " << T.Content
+ << ", Kind: " << tagKindToString(T.TagKind) << "\n");
if (T.TagKind == Tag::Kind::Triple) {
Tokens.emplace_back(T.FullMatch.str(), "&" + T.Content.str(), '&');
- LLVM_DEBUG(dbgs() << " Created UnescapeVariable token.\n");
return std::nullopt;
}
StringRef Interpolated = T.Content;
@@ -386,7 +417,6 @@ processTag(const Tag &T, SmallVectorImpl<Token> &Tokens) {
if (!Interpolated.trim().starts_with("=")) {
char Front = Interpolated.empty() ? ' ' : Interpolated.trim().front();
Tokens.emplace_back(RawBody, Interpolated.str(), Front);
- LLVM_DEBUG(dbgs() << " Created tag token of type '" << Front << "'\n");
return std::nullopt;
}
Tokens.emplace_back(RawBody, Interpolated.str(), '=');
@@ -396,8 +426,8 @@ processTag(const Tag &T, SmallVectorImpl<Token> &Tokens) {
DelimSpec = DelimSpec.trim();
auto [NewOpen, NewClose] = DelimSpec.split(' ');
- LLVM_DEBUG(dbgs() << " Found Set Delimiter tag. NewOpen='" << NewOpen
- << "', NewClose='" << NewClose << "'\n");
+ LLVM_DEBUG(dbgs() << "[Set Delimiter] NewOpen: " << NewOpen
+ << ", NewClose: " << NewClose << "\n");
return std::make_pair(NewOpen, NewClose);
}
@@ -406,22 +436,20 @@ processTag(const Tag &T, SmallVectorImpl<Token> &Tokens) {
// but we don't support that here. An unescape variable
// is represented only by {{& variable}}.
static SmallVector<Token> tokenize(StringRef Template) {
- LLVM_DEBUG(dbgs() << "Tokenizing template: \"" << Template << "\"\n");
+ LLVM_DEBUG(dbgs() << "[Tokenize Template] \"" << Template << "\"\n");
SmallVector<Token> Tokens;
SmallString<8> Open("{{");
SmallString<8> Close("}}");
size_t Start = 0;
while (Start < Template.size()) {
- LLVM_DEBUG(dbgs() << "Loop start. Start=" << Start << ", Open='" << Open
+ LLVM_DEBUG(dbgs() << "[Tokenize Loop] Start=" << Start << ", Open='" << Open
<< "', Close='" << Close << "'\n");
Tag T = findNextTag(Template, Start, Open, Close);
if (T.TagKind == Tag::Kind::None) {
// No more tags, the rest is text.
Tokens.emplace_back(Template.substr(Start).str());
- LLVM_DEBUG(dbgs() << " No more tags. Created final Text token: \""
- << Template.substr(Start) << "\"\n");
break;
}
@@ -429,7 +457,6 @@ static SmallVector<Token> tokenize(StringRef Template) {
if (T.StartPosition > Start) {
StringRef Text = Template.substr(Start, T.StartPosition - Start);
Tokens.emplace_back(Text.str());
- LLVM_DEBUG(dbgs() << " Created Text token: \"" << Text << "\"\n");
}
if (auto NewDelims = processTag(T, Tokens)) {
@@ -480,15 +507,13 @@ static SmallVector<Token> tokenize(StringRef Template) {
if ((!HasTextBehind && !HasTextAhead) || (!HasTextBehind && Idx == LastIdx))
stripTokenBefore(Tokens, Idx, CurrentToken, CurrentType);
}
- LLVM_DEBUG(dbgs() << "Tokenizing finished.\n");
return Tokens;
}
// Custom stream to escape strings.
class EscapeStringStream : public MustacheOutputStream {
public:
- explicit EscapeStringStream(raw_ostream &WrappedStream,
- EscapeMap &Escape)
+ explicit EscapeStringStream(raw_ostream &WrappedStream, EscapeMap &Escape)
: Escape(Escape), WrappedStream(WrappedStream) {
SetUnbuffered();
}
@@ -532,7 +557,7 @@ class AddIndentationStringStream : public MustacheOutputStream {
Indent.resize(Indentation, ' ');
for (char C : Data) {
- LLVM_DEBUG(dbgs() << "IndentationStream: NeedsIndent=" << NeedsIndent
+ LLVM_DEBUG(dbgs() << "[IndentationStream] NeedsIndent=" << NeedsIndent
<< ", C='" << C << "', Indentation=" << Indentation
<< "\n");
if (NeedsIndent && C != '\n') {
@@ -641,7 +666,8 @@ void Parser::parseMustache(ASTNode *Parent) {
}
}
static void toMustacheString(const json::Value &Data, raw_ostream &OS) {
- LLVM_DEBUG(dbgs() << "toMustacheString: kind=" << (int)Data.kind() << "\n");
+ LLVM_DEBUG(dbgs() << "[To Mustache String] Kind: " << jsonKindToString(Data.kind())
+ << ", Data: " << Data << "\n");
switch (Data.kind()) {
case json::Value::Null:
return;
@@ -654,7 +680,6 @@ static void toMustacheString(const json::Value &Data, raw_ostream &OS) {
}
case json::Value::String: {
auto Str = *Data.getAsString();
- LLVM_DEBUG(dbgs() << " --> writing string: \"" << Str << "\"\n");
OS << Str.str();
return;
}
@@ -674,21 +699,24 @@ static void toMustacheString(const json::Value &Data, raw_ostream &OS) {
}
}
-void ASTNode::renderRoot(const json::Value &CurrentCtx, MustacheOutputStream &OS) {
+void ASTNode::renderRoot(const json::Value &CurrentCtx,
+ MustacheOutputStream &OS) {
renderChild(CurrentCtx, OS);
}
void ASTNode::renderText(MustacheOutputStream &OS) { OS << Body; }
-void ASTNode::renderPartial(const json::Value &CurrentCtx, MustacheOutputStream &OS) {
- LLVM_DEBUG(dbgs() << "renderPartial: Accessor=" << AccessorValue[0]
+void ASTNode::renderPartial(const json::Value &CurrentCtx,
+ MustacheOutputStream &OS) {
+ LLVM_DEBUG(dbgs() << "[Render Partial] Accessor=" << AccessorValue[0]
<< ", Indentation=" << Indentation << "\n");
auto Partial = Ctx.Partials.find(AccessorValue[0]);
if (Partial != Ctx.Partials.end())
renderPartial(CurrentCtx, OS, Partial->getValue().get());
}
-void ASTNode::renderVariable(const json::Value &CurrentCtx, MustacheOutputStream &OS) {
+void ASTNode::renderVariable(const json::Value &CurrentCtx,
+ MustacheOutputStream &OS) {
auto Lambda = Ctx.Lambdas.find(AccessorValue[0]);
if (Lambda != Ctx.Lambdas.end()) {
renderLambdas(CurrentCtx, OS, Lambda->getValue());
@@ -700,13 +728,12 @@ void ASTNode::renderVariable(const json::Value &CurrentCtx, MustacheOutputStream
void ASTNode::renderUnescapeVariable(const json::Value &CurrentCtx,
MustacheOutputStream &OS) {
- LLVM_DEBUG(dbgs() << "renderUnescapeVariable: Accessor=" << AccessorValue[0]
+ LLVM_DEBUG(dbgs() << "[Render UnescapeVariable] Accessor=" << AccessorValue[0]
<< "\n");
auto Lambda = Ctx.Lambdas.find(AccessorValue[0]);
if (Lambda != Ctx.Lambdas.end()) {
renderLambdas(CurrentCtx, OS, Lambda->getValue());
} else if (const json::Value *ContextPtr = findContext()) {
- LLVM_DEBUG(dbgs() << " --> Found context value, writing to stream.\n");
OS.suspendIndentation();
toMustacheString(*ContextPtr, OS);
OS.resumeIndentation();
@@ -714,7 +741,7 @@ void ASTNode::renderUnescapeVariable(const json::Value &CurrentCtx,
}
void ASTNode::renderSection(const json::Value &CurrentCtx,
- MustacheOutputStream &OS) {
+ MustacheOutputStream &OS) {
auto SectionLambda = Ctx.SectionLambdas.find(AccessorValue[0]);
if (SectionLambda != Ctx.SectionLambdas.end()) {
renderSectionLambdas(CurrentCtx, OS, SectionLambda->getValue());
@@ -776,8 +803,6 @@ void ASTNode::render(const llvm::json::Value &Data, MustacheOutputStream &OS) {
}
const json::Value *ASTNode::findContext() {
- LLVM_DEBUG(dbgs() << "findContext: AccessorValue[0]=" << AccessorValue[0]
- << "\n");
// The mustache spec allows for dot notation to access nested values
// a single dot refers to the current context.
// We attempt to find the JSON context in the current node, if it is not
@@ -792,24 +817,12 @@ const json::Value *ASTNode::findContext() {
StringRef CurrentAccessor = AccessorValue[0];
ASTNode *CurrentParent = Parent;
- LLVM_DEBUG(dbgs() << "findContext: ParentContext: ";
- if (ParentContext) ParentContext->print(dbgs());
- else dbgs() << "nullptr";
- dbgs() << "\n");
-
while (!CurrentContext || !CurrentContext->get(CurrentAccessor)) {
- LLVM_DEBUG(dbgs() << "findContext: climbing parent\n");
if (CurrentParent->Ty != Root) {
CurrentContext = CurrentParent->ParentContext->getAsObject();
CurrentParent = CurrentParent->Parent;
- LLVM_DEBUG(dbgs() << "findContext: new ParentContext: ";
- if (CurrentParent->ParentContext)
- CurrentParent->ParentContext->print(dbgs());
- else dbgs() << "nullptr";
- dbgs() << "\n");
continue;
}
- LLVM_DEBUG(dbgs() << "findContext: reached root, not found\n");
return nullptr;
}
const json::Value *Context = nullptr;
@@ -825,28 +838,24 @@ const json::Value *ASTNode::findContext() {
Context = CurrentValue;
}
}
- LLVM_DEBUG(dbgs() << "findContext: found value: ";
- if (Context) Context->print(dbgs());
- else dbgs() << "nullptr";
- dbgs() << "\n");
return Context;
}
-void ASTNode::renderChild(const json::Value &Contexts, MustacheOutputStream &OS) {
+void ASTNode::renderChild(const json::Value &Contexts,
+ MustacheOutputStream &OS) {
for (AstPtr &Child : Children)
Child->render(Contexts, OS);
}
-void ASTNode::renderPartial(const json::Value &Contexts, MustacheOutputStream &OS,
- ASTNode *Partial) {
- LLVM_DEBUG(dbgs() << "renderPartial (helper): Indentation=" << Indentation
- << "\n");
+void ASTNode::renderPartial(const json::Value &Contexts,
+ MustacheOutputStream &OS, ASTNode *Partial) {
+ LLVM_DEBUG(dbgs() << "[Render Partial Indentation] " << Indentation << "\n");
AddIndentationStringStream IS(OS, Indentation);
Partial->render(Contexts, IS);
}
-void ASTNode::renderLambdas(const json::Value &Contexts, MustacheOutputStream &OS,
- Lambda &L) {
+void ASTNode::renderLambdas(const json::Value &Contexts,
+ MustacheOutputStream &OS, Lambda &L) {
json::Value LambdaResult = L();
std::string LambdaStr;
raw_string_ostream Output(LambdaStr);
|
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions cpp -- llvm/lib/Support/Mustache.cpp
View the diff from clang-format here.diff --git a/llvm/lib/Support/Mustache.cpp b/llvm/lib/Support/Mustache.cpp
index 178f97030..0098b1d39 100644
--- a/llvm/lib/Support/Mustache.cpp
+++ b/llvm/lib/Support/Mustache.cpp
@@ -864,7 +864,8 @@ void ASTNode::renderChild(const json::Value &Contexts,
void ASTNode::renderPartial(const json::Value &Contexts,
MustacheOutputStream &OS, ASTNode *Partial) {
- LLVM_DEBUG(dbgs() << "[Render Partial Indentation] Indentation: " << Indentation << "\n");
+ LLVM_DEBUG(dbgs() << "[Render Partial Indentation] Indentation: "
+ << Indentation << "\n");
AddIndentationStringStream IS(OS, Indentation);
Partial->render(Contexts, IS);
}
|
a5faef5
to
e0974e4
Compare
ddcaf16
to
e89315c
Compare
e0974e4
to
d5312d1
Compare
e89315c
to
68c27a3
Compare
d5312d1
to
56b63ac
Compare
68c27a3
to
c2719fb
Compare
56b63ac
to
2819519
Compare
f153c6a
to
f4b4bc8
Compare
078165d
to
a211ef4
Compare
f4b4bc8
to
df32a14
Compare
340e2ef
to
f2fdb9c
Compare
df32a14
to
b22fb3e
Compare
bd78555
to
473adbc
Compare
b22fb3e
to
79eaa0a
Compare
llvm/lib/Support/Mustache.cpp
Outdated
|
||
for (char C : Data) { | ||
LLVM_DEBUG(dbgs() << "IndentationStream: NeedsIndent=" << NeedsIndent | ||
LLVM_DEBUG(dbgs() << "[IndentationStream] NeedsIndent=" << NeedsIndent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not clear to me where we use FooBar
and when we use Foo Bar
since there doesn't seem to be much consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this one is the odd one out. good catch. let me shore up the rest.
79eaa0a
to
eb3ba40
Compare
size_t StartPosition = StringRef::npos; | ||
}; | ||
|
||
static const char *tagKindToString(Tag::Kind K) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ilovepi ,
These two new functions are unused in builds without asserts:
../lib/Support/Mustache.cpp:332:20: error: unused function 'tagKindToString' [-Werror,-Wunused-function]
332 | static const char *tagKindToString(Tag::Kind K) {
| ^~~~~~~~~~~~~~~
../lib/Support/Mustache.cpp:344:20: error: unused function 'jsonKindToString' [-Werror,-Wunused-function]
344 | static const char *jsonKindToString(json::Value::Kind K) {
| ^~~~~~~~~~~~~~~~
2 errors generated.
The existing logging was inconsistent, and we logged too many things. This PR introduces a more principled schema, and eliminates many, redundant log lines.
No description provided.