[PATCH] YAML: Add an optional 'flow' field to the mapping trait to allow flow mapping output.
Justin Bogner
mail at justinbogner.com
Fri May 1 17:21:11 PDT 2015
Alex Lorenz <arphaman at gmail.com> writes:
> Hi bogner, bob.wilson,
>
> This patch adds an optional flow field to the MappingTrait class that
> enables yaml IO to output flow mappings.
> I have a patch that updates the documentation as well, I will post it later.
Please add the documentation in the same commit as the change.
> REPOSITORY
> rL LLVM
>
> http://reviews.llvm.org/D9450
>
> Files:
> include/llvm/Support/YAMLTraits.h
> lib/Support/YAMLTraits.cpp
> unittests/Support/YAMLIOTest.cpp
>
> EMAIL PREFERENCES
> http://reviews.llvm.org/settings/panel/emailpreferences/
>
> Index: include/llvm/Support/YAMLTraits.h
> ===================================================================
> --- include/llvm/Support/YAMLTraits.h
> +++ include/llvm/Support/YAMLTraits.h
> @@ -46,6 +46,10 @@
> // static void mapping(IO &io, T &fields);
> // Optionally may provide:
> // static StringRef validate(IO &io, T &fields);
> + //
> + // The optional flow flag will cause generated YAML to use a flow mapping
> + // (e.g. { a: 0, b: 1 }):
> + // static const bool flow = true;
> };
>
>
> @@ -445,6 +449,9 @@
> virtual bool preflightKey(const char*, bool, bool, bool &, void *&) = 0;
> virtual void postflightKey(void*) = 0;
>
> + virtual void beginFlowMapping() = 0;
> + virtual void endFlowMapping() = 0;
> +
> virtual void beginEnumScalar() = 0;
> virtual bool matchEnumScalar(const char*, bool) = 0;
> virtual bool matchEnumFallback() = 0;
> @@ -643,7 +650,10 @@
> template<typename T>
> typename std::enable_if<validatedMappingTraits<T>::value, void>::type
> yamlize(IO &io, T &Val, bool) {
> - io.beginMapping();
> + if (has_FlowTraits<MappingTraits<T>>::value)
> + io.beginFlowMapping();
> + else
> + io.beginMapping();
> if (io.outputting()) {
> StringRef Err = MappingTraits<T>::validate(io, Val);
> if (!Err.empty()) {
> @@ -657,15 +667,24 @@
> if (!Err.empty())
> io.setError(Err);
> }
> - io.endMapping();
> + if (has_FlowTraits<MappingTraits<T>>::value)
> + io.endFlowMapping();
> + else
> + io.endMapping();
> }
>
> template<typename T>
> typename std::enable_if<unvalidatedMappingTraits<T>::value, void>::type
> yamlize(IO &io, T &Val, bool) {
> - io.beginMapping();
> - MappingTraits<T>::mapping(io, Val);
> - io.endMapping();
> + if (has_FlowTraits<MappingTraits<T>>::value) {
> + io.beginFlowMapping();
> + MappingTraits<T>::mapping(io, Val);
> + io.endFlowMapping();
> + } else {
> + io.beginMapping();
> + MappingTraits<T>::mapping(io, Val);
> + io.endMapping();
> + }
I'm not a big fan of the "check the flow traits at all of the callers"
style that's prevalent here, but it seems to be consistent with the
existing code for FlowElements so I guess it makes sense to do it this
way. More below.
> }
>
> template<typename T>
> @@ -900,6 +919,8 @@
> void endMapping() override;
> bool preflightKey(const char *, bool, bool, bool &, void *&) override;
> void postflightKey(void *) override;
> + void beginFlowMapping() override;
> + void endFlowMapping() override;
> unsigned beginSequence() override;
> void endSequence() override;
> bool preflightElement(unsigned index, void *&) override;
> @@ -1028,6 +1049,8 @@
> void endMapping() override;
> bool preflightKey(const char *key, bool, bool, bool &, void *&) override;
> void postflightKey(void *) override;
> + void beginFlowMapping() override;
> + void endFlowMapping() override;
> unsigned beginSequence() override;
> void endSequence() override;
> bool preflightElement(unsigned, void *&) override;
> @@ -1061,12 +1084,20 @@
> void outputNewLine();
> void paddedKey(StringRef key);
>
> - enum InState { inSeq, inFlowSeq, inMapFirstKey, inMapOtherKey };
> + enum InState {
> + inSeq,
> + inFlowSeq,
> + inMapFirstKey,
> + inMapOtherKey,
> + inFlowMapFirstKey,
> + inFlowMapOtherKey
> + };
>
> llvm::raw_ostream &Out;
> SmallVector<InState, 8> StateStack;
> int Column;
> int ColumnAtFlowStart;
> + int ColumnAtMapFlowStart;
> bool NeedBitValueComma;
> bool NeedFlowSequenceComma;
> bool EnumerationMatchFound;
> Index: lib/Support/YAMLTraits.cpp
> ===================================================================
> --- lib/Support/YAMLTraits.cpp
> +++ lib/Support/YAMLTraits.cpp
> @@ -168,6 +168,10 @@
> }
> }
>
> +void Input::beginFlowMapping() { beginMapping(); }
> +
> +void Input::endFlowMapping() { endMapping(); }
> +
> unsigned Input::beginSequence() {
> if (SequenceHNode *SQ = dyn_cast<SequenceHNode>(CurrentNode))
> return SQ->Entries.size();
> @@ -393,6 +397,7 @@
> Out(yout),
> Column(0),
> ColumnAtFlowStart(0),
> + ColumnAtMapFlowStart(0),
> NeedBitValueComma(false),
> NeedFlowSequenceComma(false),
> EnumerationMatchFound(false),
> @@ -427,8 +432,23 @@
> bool &UseDefault, void *&) {
> UseDefault = false;
> if (Required || !SameAsDefault) {
> - this->newLineCheck();
> - this->paddedKey(Key);
> + auto State = StateStack.back();
> + if (State == inFlowMapFirstKey || State == inFlowMapOtherKey) {
> + if (State == inFlowMapOtherKey)
> + output(", ");
> + if (Column > 70) {
> + output("\n");
> + for (int i = 0; i < ColumnAtMapFlowStart; ++i)
> + output(" ");
> + Column = ColumnAtMapFlowStart;
> + output(" ");
> + }
> + output(Key);
> + output(": ");
> + } else {
> + this->newLineCheck();
> + this->paddedKey(Key);
> + }
So to be consistent this should really be split into preflightKey and
preflightFlowKey, much like is done for elements. As it is here, there's
a big difference in abstraction level between the two branches of the
if, which makes this hard to follow.
> return true;
> }
> return false;
> @@ -438,9 +458,24 @@
> if (StateStack.back() == inMapFirstKey) {
> StateStack.pop_back();
> StateStack.push_back(inMapOtherKey);
> + } else if (StateStack.back() == inFlowMapFirstKey) {
> + StateStack.pop_back();
> + StateStack.push_back(inFlowMapOtherKey);
> }
> }
>
> +void Output::beginFlowMapping() {
> + StateStack.push_back(inFlowMapFirstKey);
> + this->newLineCheck();
> + ColumnAtMapFlowStart = Column;
> + output("{ ");
> +}
> +
> +void Output::endFlowMapping() {
> + StateStack.pop_back();
> + this->outputUpToEndOfLine(" }");
> +}
> +
> void Output::beginDocuments() {
> this->outputUpToEndOfLine("---");
> }
> @@ -607,7 +642,9 @@
>
> void Output::outputUpToEndOfLine(StringRef s) {
> this->output(s);
> - if (StateStack.empty() || StateStack.back() != inFlowSeq)
> + if (StateStack.empty() || (StateStack.back() != inFlowSeq &&
> + StateStack.back() != inFlowMapFirstKey &&
> + StateStack.back() != inFlowMapOtherKey))
> NeedsNewLine = true;
> }
>
> @@ -634,7 +671,8 @@
> if (StateStack.back() == inSeq) {
> OutputDash = true;
> } else if ((StateStack.size() > 1) && ((StateStack.back() == inMapFirstKey) ||
> - (StateStack.back() == inFlowSeq)) &&
> + (StateStack.back() == inFlowSeq) ||
> + (StateStack.back() == inFlowMapFirstKey)) &&
> (StateStack[StateStack.size() - 2] == inSeq)) {
> --Indent;
> OutputDash = true;
> Index: unittests/Support/YAMLIOTest.cpp
> ===================================================================
> --- unittests/Support/YAMLIOTest.cpp
> +++ unittests/Support/YAMLIOTest.cpp
> @@ -1367,6 +1367,91 @@
> EXPECT_TRUE(!!yin.error());
> }
>
> +//===----------------------------------------------------------------------===//
> +// Test flow mapping
> +//===----------------------------------------------------------------------===//
> +
> +struct FlowFooBar {
> + int foo;
> + int bar;
> +
> + FlowFooBar() : foo(0), bar(0) {}
> + FlowFooBar(int foo, int bar) : foo(foo), bar(bar) {}
> +};
> +
> +typedef std::vector<FlowFooBar> FlowFooBarSequence;
> +
> +LLVM_YAML_IS_SEQUENCE_VECTOR(FlowFooBar)
> +
> +struct FlowFooBarDoc {
> + FlowFooBar attribute;
> + FlowFooBarSequence seq;
> +};
> +
> +namespace llvm {
> +namespace yaml {
> + template <>
> + struct MappingTraits<FlowFooBar> {
> + static void mapping(IO &io, FlowFooBar &fb) {
> + io.mapRequired("foo", fb.foo);
> + io.mapRequired("bar", fb.bar);
> + }
> +
> + static const bool flow = true;
> + };
> +
> + template <>
> + struct MappingTraits<FlowFooBarDoc> {
> + static void mapping(IO &io, FlowFooBarDoc &fb) {
> + io.mapRequired("attribute", fb.attribute);
> + io.mapRequired("seq", fb.seq);
> + }
> + };
> +}
> +}
> +
> +//
> +// Test writing then reading back custom mappings
> +//
> +TEST(YAMLIO, TestReadWriteMyFlowMapping) {
> + std::string intermediate;
> + {
> + FlowFooBarDoc doc;
> + doc.attribute = FlowFooBar(42, 907);
> + doc.seq.push_back(FlowFooBar(1, 2));
> + doc.seq.push_back(FlowFooBar(0, 0));
> + doc.seq.push_back(FlowFooBar(-1, 1024));
> +
> + llvm::raw_string_ostream ostr(intermediate);
> + Output yout(ostr);
> + yout << doc;
> +
> + // Verify that mappings were written in flow style
> + ostr.flush();
> + llvm::StringRef flowOut(intermediate);
> + EXPECT_NE(llvm::StringRef::npos, flowOut.find("{ foo: 42, bar: 907 }"));
> + EXPECT_NE(llvm::StringRef::npos, flowOut.find("- { foo: 1, bar: 2 }"));
> + EXPECT_NE(llvm::StringRef::npos, flowOut.find("- { foo: 0, bar: 0 }"));
> + EXPECT_NE(llvm::StringRef::npos, flowOut.find("- { foo: -1, bar: 1024 }"));
> + }
> +
> + {
> + Input yin(intermediate);
> + FlowFooBarDoc doc2;
> + yin >> doc2;
> +
> + EXPECT_FALSE(yin.error());
> + EXPECT_EQ(doc2.attribute.foo, 42);
> + EXPECT_EQ(doc2.attribute.bar, 907);
> + EXPECT_EQ(doc2.seq.size(), 3UL);
> + EXPECT_EQ(doc2.seq[0].foo, 1);
> + EXPECT_EQ(doc2.seq[0].bar, 2);
> + EXPECT_EQ(doc2.seq[1].foo, 0);
> + EXPECT_EQ(doc2.seq[1].bar, 0);
> + EXPECT_EQ(doc2.seq[2].foo, -1);
> + EXPECT_EQ(doc2.seq[2].bar, 1024);
> + }
> +}
>
> //===----------------------------------------------------------------------===//
> // Test error handling
More information about the llvm-commits
mailing list