-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[llvm][mustache] Fix failing StandaloneIndentation test #159192
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) ChangesWhen rendering partials, we need to use an indentation stream, Patch is 65.70 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/159192.diff 3 Files Affected:
diff --git a/llvm/lib/Support/Mustache.cpp b/llvm/lib/Support/Mustache.cpp
index 9dbc87a457e97..e084e379312ef 100644
--- a/llvm/lib/Support/Mustache.cpp
+++ b/llvm/lib/Support/Mustache.cpp
@@ -57,6 +57,33 @@ static Accessor splitMustacheString(StringRef Str) {
namespace llvm::mustache {
+class MustacheOutputStream : public raw_ostream {
+public:
+ MustacheOutputStream() = default;
+ ~MustacheOutputStream() override = default;
+
+ virtual void suspendIndentation() {}
+ virtual void resumeIndentation() {}
+
+private:
+ void anchor() override;
+};
+
+void MustacheOutputStream::anchor() {}
+
+class RawMustacheOutputStream : public MustacheOutputStream {
+public:
+ RawMustacheOutputStream(raw_ostream &OS) : OS(OS) { SetUnbuffered(); }
+
+private:
+ raw_ostream &OS;
+
+ void write_impl(const char *Ptr, size_t Size) override {
+ OS.write(Ptr, Size);
+ }
+ uint64_t current_pos() const override { return OS.tell(); }
+};
+
class Token {
public:
enum class Type {
@@ -157,29 +184,29 @@ class ASTNode {
void setIndentation(size_t NewIndentation) { Indentation = NewIndentation; };
- void render(const llvm::json::Value &Data, llvm::raw_ostream &OS);
+ void render(const llvm::json::Value &Data, MustacheOutputStream &OS);
private:
- void renderLambdas(const llvm::json::Value &Contexts, llvm::raw_ostream &OS,
+ void renderLambdas(const llvm::json::Value &Contexts, MustacheOutputStream &OS,
Lambda &L);
void renderSectionLambdas(const llvm::json::Value &Contexts,
- llvm::raw_ostream &OS, SectionLambda &L);
+ MustacheOutputStream &OS, SectionLambda &L);
- void renderPartial(const llvm::json::Value &Contexts, llvm::raw_ostream &OS,
+ void renderPartial(const llvm::json::Value &Contexts, MustacheOutputStream &OS,
ASTNode *Partial);
- void renderChild(const llvm::json::Value &Context, llvm::raw_ostream &OS);
+ void renderChild(const llvm::json::Value &Context, MustacheOutputStream &OS);
const llvm::json::Value *findContext();
- void renderRoot(const json::Value &CurrentCtx, raw_ostream &OS);
- void renderText(raw_ostream &OS);
- void renderPartial(const json::Value &CurrentCtx, raw_ostream &OS);
- void renderVariable(const json::Value &CurrentCtx, raw_ostream &OS);
- void renderUnescapeVariable(const json::Value &CurrentCtx, raw_ostream &OS);
- void renderSection(const json::Value &CurrentCtx, raw_ostream &OS);
- void renderInvertSection(const json::Value &CurrentCtx, raw_ostream &OS);
+ void renderRoot(const json::Value &CurrentCtx, MustacheOutputStream &OS);
+ 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 renderSection(const json::Value &CurrentCtx, MustacheOutputStream &OS);
+ void renderInvertSection(const json::Value &CurrentCtx, MustacheOutputStream &OS);
MustacheContext &Ctx;
Type Ty;
@@ -393,7 +420,7 @@ static SmallVector<Token> tokenize(StringRef Template) {
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: \""
+ LLVM_DEBUG(dbgs() << " No more tags. Created final Text token: \""
<< Template.substr(Start) << "\"\n");
break;
}
@@ -458,9 +485,9 @@ static SmallVector<Token> tokenize(StringRef Template) {
}
// Custom stream to escape strings.
-class EscapeStringStream : public raw_ostream {
+class EscapeStringStream : public MustacheOutputStream {
public:
- explicit EscapeStringStream(llvm::raw_ostream &WrappedStream,
+ explicit EscapeStringStream(raw_ostream &WrappedStream,
EscapeMap &Escape)
: Escape(Escape), WrappedStream(WrappedStream) {
SetUnbuffered();
@@ -482,19 +509,22 @@ class EscapeStringStream : public raw_ostream {
private:
EscapeMap &Escape;
- llvm::raw_ostream &WrappedStream;
+ raw_ostream &WrappedStream;
};
// Custom stream to add indentation used to for rendering partials.
-class AddIndentationStringStream : public raw_ostream {
+class AddIndentationStringStream : public MustacheOutputStream {
public:
- explicit AddIndentationStringStream(llvm::raw_ostream &WrappedStream,
+ explicit AddIndentationStringStream(raw_ostream &WrappedStream,
size_t Indentation)
: Indentation(Indentation), WrappedStream(WrappedStream),
- NeedsIndent(true) {
+ NeedsIndent(true), IsSuspended(false) {
SetUnbuffered();
}
+ void suspendIndentation() override { IsSuspended = true; }
+ void resumeIndentation() override { IsSuspended = false; }
+
protected:
void write_impl(const char *Ptr, size_t Size) override {
llvm::StringRef Data(Ptr, Size);
@@ -502,12 +532,15 @@ class AddIndentationStringStream : public raw_ostream {
Indent.resize(Indentation, ' ');
for (char C : Data) {
+ LLVM_DEBUG(dbgs() << "IndentationStream: NeedsIndent=" << NeedsIndent
+ << ", C='" << C << "', Indentation=" << Indentation
+ << "\n");
if (NeedsIndent && C != '\n') {
WrappedStream << Indent;
NeedsIndent = false;
}
WrappedStream << C;
- if (C == '\n')
+ if (C == '\n' && !IsSuspended)
NeedsIndent = true;
}
}
@@ -516,8 +549,9 @@ class AddIndentationStringStream : public raw_ostream {
private:
size_t Indentation;
- llvm::raw_ostream &WrappedStream;
+ raw_ostream &WrappedStream;
bool NeedsIndent;
+ bool IsSuspended;
};
class Parser {
@@ -607,6 +641,7 @@ void Parser::parseMustache(ASTNode *Parent) {
}
}
static void toMustacheString(const json::Value &Data, raw_ostream &OS) {
+ LLVM_DEBUG(dbgs() << "toMustacheString: kind=" << (int)Data.kind() << "\n");
switch (Data.kind()) {
case json::Value::Null:
return;
@@ -619,6 +654,7 @@ 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;
}
@@ -638,19 +674,21 @@ static void toMustacheString(const json::Value &Data, raw_ostream &OS) {
}
}
-void ASTNode::renderRoot(const json::Value &CurrentCtx, raw_ostream &OS) {
+void ASTNode::renderRoot(const json::Value &CurrentCtx, MustacheOutputStream &OS) {
renderChild(CurrentCtx, OS);
}
-void ASTNode::renderText(raw_ostream &OS) { OS << Body; }
+void ASTNode::renderText(MustacheOutputStream &OS) { OS << Body; }
-void ASTNode::renderPartial(const json::Value &CurrentCtx, raw_ostream &OS) {
+void ASTNode::renderPartial(const json::Value &CurrentCtx, MustacheOutputStream &OS) {
+ LLVM_DEBUG(dbgs() << "renderPartial: 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, raw_ostream &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());
@@ -661,16 +699,22 @@ void ASTNode::renderVariable(const json::Value &CurrentCtx, raw_ostream &OS) {
}
void ASTNode::renderUnescapeVariable(const json::Value &CurrentCtx,
- raw_ostream &OS) {
+ MustacheOutputStream &OS) {
+ LLVM_DEBUG(dbgs() << "renderUnescapeVariable: 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();
}
}
-void ASTNode::renderSection(const json::Value &CurrentCtx, raw_ostream &OS) {
+void ASTNode::renderSection(const json::Value &CurrentCtx,
+ MustacheOutputStream &OS) {
auto SectionLambda = Ctx.SectionLambdas.find(AccessorValue[0]);
if (SectionLambda != Ctx.SectionLambdas.end()) {
renderSectionLambdas(CurrentCtx, OS, SectionLambda->getValue());
@@ -690,7 +734,7 @@ void ASTNode::renderSection(const json::Value &CurrentCtx, raw_ostream &OS) {
}
void ASTNode::renderInvertSection(const json::Value &CurrentCtx,
- raw_ostream &OS) {
+ MustacheOutputStream &OS) {
bool IsLambda = Ctx.SectionLambdas.contains(AccessorValue[0]);
const json::Value *ContextPtr = findContext();
if (isContextFalsey(ContextPtr) && !IsLambda) {
@@ -698,40 +742,42 @@ void ASTNode::renderInvertSection(const json::Value &CurrentCtx,
}
}
-void ASTNode::render(const json::Value &CurrentCtx, raw_ostream &OS) {
+void ASTNode::render(const llvm::json::Value &Data, MustacheOutputStream &OS) {
if (Ty != Root && Ty != Text && AccessorValue.empty())
return;
// Set the parent context to the incoming context so that we
// can walk up the context tree correctly in findContext().
- ParentContext = &CurrentCtx;
+ ParentContext = &Data;
switch (Ty) {
case Root:
- renderRoot(CurrentCtx, OS);
+ renderRoot(Data, OS);
return;
case Text:
renderText(OS);
return;
case Partial:
- renderPartial(CurrentCtx, OS);
+ renderPartial(Data, OS);
return;
case Variable:
- renderVariable(CurrentCtx, OS);
+ renderVariable(Data, OS);
return;
case UnescapeVariable:
- renderUnescapeVariable(CurrentCtx, OS);
+ renderUnescapeVariable(Data, OS);
return;
case Section:
- renderSection(CurrentCtx, OS);
+ renderSection(Data, OS);
return;
case InvertSection:
- renderInvertSection(CurrentCtx, OS);
+ renderInvertSection(Data, OS);
return;
}
llvm_unreachable("Invalid ASTNode type");
}
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
@@ -746,12 +792,24 @@ 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;
@@ -767,21 +825,27 @@ 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, llvm::raw_ostream &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, llvm::raw_ostream &OS,
+void ASTNode::renderPartial(const json::Value &Contexts, MustacheOutputStream &OS,
ASTNode *Partial) {
+ LLVM_DEBUG(dbgs() << "renderPartial (helper): Indentation=" << Indentation
+ << "\n");
AddIndentationStringStream IS(OS, Indentation);
Partial->render(Contexts, IS);
}
-void ASTNode::renderLambdas(const json::Value &Contexts, llvm::raw_ostream &OS,
+void ASTNode::renderLambdas(const json::Value &Contexts, MustacheOutputStream &OS,
Lambda &L) {
json::Value LambdaResult = L();
std::string LambdaStr;
@@ -799,7 +863,7 @@ void ASTNode::renderLambdas(const json::Value &Contexts, llvm::raw_ostream &OS,
}
void ASTNode::renderSectionLambdas(const json::Value &Contexts,
- llvm::raw_ostream &OS, SectionLambda &L) {
+ MustacheOutputStream &OS, SectionLambda &L) {
json::Value Return = L(RawBody);
if (isFalsey(Return))
return;
@@ -812,7 +876,8 @@ void ASTNode::renderSectionLambdas(const json::Value &Contexts,
}
void Template::render(const json::Value &Data, llvm::raw_ostream &OS) {
- Tree->render(Data, OS);
+ RawMustacheOutputStream MOS(OS);
+ Tree->render(Data, MOS);
}
void Template::registerPartial(std::string Name, std::string Partial) {
diff --git a/llvm/unittests/Support/MustacheTest.cpp b/llvm/unittests/Support/MustacheTest.cpp
index addf0355c4d0a..b98c218796b0b 100644
--- a/llvm/unittests/Support/MustacheTest.cpp
+++ b/llvm/unittests/Support/MustacheTest.cpp
@@ -22,7 +22,7 @@ using namespace llvm::json;
TEST(MustacheInterpolation, NoInterpolation) {
// Mustache-free templates should render as-is.
Value D = {};
- auto T = Template("Hello from {Mustache}!\n");
+ Template T("Hello from {Mustache}!\n");
std::string Out;
raw_string_ostream OS(Out);
T.render(D, OS);
@@ -32,7 +32,7 @@ TEST(MustacheInterpolation, NoInterpolation) {
TEST(MustacheInterpolation, BasicInterpolation) {
// Unadorned tags should interpolate content into the template.
Value D = Object{{"subject", "World"}};
- auto T = Template("Hello, {{subject}}!");
+ Template T("Hello, {{subject}}!");
std::string Out;
raw_string_ostream OS(Out);
T.render(D, OS);
@@ -42,7 +42,7 @@ TEST(MustacheInterpolation, BasicInterpolation) {
TEST(MustacheInterpolation, NoReinterpolation) {
// Interpolated tag output should not be re-interpolated.
Value D = Object{{"template", "{{planet}}"}, {"planet", "Earth"}};
- auto T = Template("{{template}}: {{planet}}");
+ Template T("{{template}}: {{planet}}");
std::string Out;
raw_string_ostream OS(Out);
T.render(D, OS);
@@ -54,7 +54,7 @@ TEST(MustacheInterpolation, HTMLEscaping) {
Value D = Object{
{"forbidden", "& \" < >"},
};
- auto T = Template("These characters should be HTML escaped: {{forbidden}}\n");
+ Template T("These characters should be HTML escaped: {{forbidden}}\n");
std::string Out;
raw_string_ostream OS(Out);
T.render(D, OS);
@@ -67,8 +67,7 @@ TEST(MustacheInterpolation, Ampersand) {
Value D = Object{
{"forbidden", "& \" < >"},
};
- auto T =
- Template("These characters should not be HTML escaped: {{&forbidden}}\n");
+ Template T("These characters should not be HTML escaped: {{&forbidden}}\n");
std::string Out;
raw_string_ostream OS(Out);
T.render(D, OS);
@@ -78,7 +77,7 @@ TEST(MustacheInterpolation, Ampersand) {
TEST(MustacheInterpolation, BasicIntegerInterpolation) {
// Integers should interpolate seamlessly.
Value D = Object{{"mph", 85}};
- auto T = Template("{{mph}} miles an hour!");
+ Template T("{{mph}} miles an hour!");
std::string Out;
raw_string_ostream OS(Out);
T.render(D, OS);
@@ -88,7 +87,7 @@ TEST(MustacheInterpolation, BasicIntegerInterpolation) {
TEST(MustacheInterpolation, AmpersandIntegerInterpolation) {
// Integers should interpolate seamlessly.
Value D = Object{{"mph", 85}};
- auto T = Template("{{&mph}} miles an hour!");
+ Template T("{{&mph}} miles an hour!");
std::string Out;
raw_string_ostream OS(Out);
T.render(D, OS);
@@ -98,7 +97,7 @@ TEST(MustacheInterpolation, AmpersandIntegerInterpolation) {
TEST(MustacheInterpolation, BasicDecimalInterpolation) {
// Decimals should interpolate seamlessly with proper significance.
Value D = Object{{"power", 1.21}};
- auto T = Template("{{power}} jiggawatts!");
+ Template T("{{power}} jiggawatts!");
std::string Out;
raw_string_ostream OS(Out);
T.render(D, OS);
@@ -108,7 +107,7 @@ TEST(MustacheInterpolation, BasicDecimalInterpolation) {
TEST(MustacheInterpolation, BasicNullInterpolation) {
// Nulls should interpolate as the empty string.
Value D = Object{{"cannot", nullptr}};
- auto T = Template("I ({{cannot}}) be seen!");
+ Template T("I ({{cannot}}) be seen!");
std::string Out;
raw_string_ostream OS(Out);
T.render(D, OS);
@@ -118,7 +117,7 @@ TEST(MustacheInterpolation, BasicNullInterpolation) {
TEST(MustacheInterpolation, AmpersandNullInterpolation) {
// Nulls should interpolate as the empty string.
Value D = Object{{"cannot", nullptr}};
- auto T = Template("I ({{&cannot}}) be seen!");
+ Template T("I ({{&cannot}}) be seen!");
std::string Out;
raw_string_ostream OS(Out);
T.render(D, OS);
@@ -128,7 +127,7 @@ TEST(MustacheInterpolation, AmpersandNullInterpolation) {
TEST(MustacheInterpolation, BasicContextMissInterpolation) {
// Failed context lookups should default to empty strings.
Value D = Object{};
- auto T = Template("I ({{cannot}}) be seen!");
+ Template T("I ({{cannot}}) be seen!");
std::string Out;
raw_string_ostream OS(Out);
T.render(D, OS);
@@ -138,7 +137,7 @@ TEST(MustacheInterpolation, BasicContextMissInterpolation) {
TEST(MustacheInterpolation, DottedNamesBasicInterpolation) {
// Dotted names should be considered a form of shorthand for sections.
Value D = Object{{"person", Object{{"name", "Joe"}}}};
- auto T = Template("{{person.name}} == {{#person}}{{name}}{{/person}}");
+ Template T("{{person.name}} == {{#person}}{{name}}{{/person}}");
std::string Out;
raw_string_ostream OS(Out);
T.render(D, OS);
@@ -148,7 +147,7 @@ TEST(MustacheInterpolation, DottedNamesBasicInterpolation) {
TEST(MustacheInterpolation, DottedNamesAmpersandInterpolation) {
// Dotted names should be considered a form of shorthand for sections.
Value D = Object{{"person", Object{{"name", "Joe"}}}};
- auto T = Template("{{&person.name}} == {{#person}}{{&name}}{{/person}}");
+ Template T("{{&person.name}} == {{#person}}{{&name}}{{/person}}");
std::string Out;
raw_string_ostream OS(Out);
T.render(D, OS);
@@ -163,7 +162,7 @@ TEST(MustacheInterpolation, DottedNamesArbitraryDepth) {
Object{{"c",
Object{{"d",
Object{{"e", Object{{"name", "Phil"}}}}}}}}}}}};
- auto T = Template("{{a.b.c.d.e.name}}");
+ Template T("{{a.b.c.d.e.name}}");
std::string Out;
raw_string_ostream OS(Out);
T.render(D, OS);
@@ -173,7 +172,7 @@ TEST(MustacheInterpolation, DottedNamesArbitraryDepth) {
TEST(MustacheInterpolation, DottedNamesBrokenChains) {
// Any falsey value prior to the last part of the name should yield ''.
Value D = Object{{"a", Object{}}};
- auto T = Template("{{a.b.c}} == ");
+ Template T("{{a.b.c}} == ");
std::string Out;
raw_string_ostream OS(Out);
T.render(D, OS);
@@ -184,7 +183,7 @@ TEST(MustacheInterpolation, DottedNamesBrokenChainResolution) {
// Each part of a dotted name should resolve only against its parent.
Value D =
Object{{"a", Object{{"b", Object{}}}}, {"c", Object{{"name", "Jim"}}}};
- auto T = Template("{{a.b.c.name}} == ");
+ Template T("{{a.b.c.name}} == ");
std::string Out;
raw_string_ostream OS(Out);
T.render(D, OS);
@@ -201,7 +200,7 @@ TEST(MustacheI...
[truncated]
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
cbc8872
to
f441a1c
Compare
e0974e4
to
d5312d1
Compare
a31a1ef
to
de12e19
Compare
56b63ac
to
2819519
Compare
1b4ad5d
to
d0ee769
Compare
078165d
to
a211ef4
Compare
446d204
to
9e5c507
Compare
a211ef4
to
340e2ef
Compare
f2fdb9c
to
bd78555
Compare
When rendering partials, we need to use an indentation stream, but when part of the partial is a unescaped sequence, we cannot indent those. To address this, we build a common MustacheStream interface for all the output streams to use. This allows us to further customize the AddIndentationStream implementation and opt it out of indenting the UnescapeSequence.
bd78555
to
473adbc
Compare
When rendering partials, we need to use an indentation stream, but when part of the partial is a unescaped sequence, we cannot indent those. To address this, we build a common MustacheStream interface for all the output streams to use. This allows us to further customize the AddIndentationStream implementation and opt it out of indenting the UnescapeSequence.
When rendering partials, we need to use an indentation stream,
but when part of the partial is a unescaped sequence, we cannot
indent those. To address this, we build a common MustacheStream
interface for all the output streams to use. This allows us to
further customize the AddIndentationStream implementation
and opt it out of indenting the UnescapeSequence.