[PATCH] YAML: Add an optional 'flow' field to the mapping trait to allow flow mapping output.

Justin Bogner mail at justinbogner.com
Mon May 4 12:47:22 PDT 2015


Alex Lorenz <arphaman at gmail.com> writes:
> Refactoring: I've extracted the output of the flow mapping key into a
> separate method as suggested by Justin.

LGTM

>
> REPOSITORY
>   rL LLVM
>
> http://reviews.llvm.org/D9450
>
> Files:
>   docs/YamlIO.rst
>   include/llvm/Support/YAMLTraits.h
>   lib/Support/YAMLTraits.cpp
>   unittests/Support/YAMLIOTest.cpp
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
> Index: docs/YamlIO.rst
> ===================================================================
> --- docs/YamlIO.rst
> +++ docs/YamlIO.rst
> @@ -723,6 +723,31 @@
>        }
>      };
>  
> +Flow Mapping
> +------------
> +A YAML "flow mapping" is a mapping that uses the inline notation
> +(e.g { x: 1, y: 0 } ) when written to YAML. To specify that a type should be
> +written in YAML using flow mapping, your MappingTraits specialization should
> +add "static const bool flow = true;". For instance:
> +
> +.. code-block:: c++
> +
> +    using llvm::yaml::MappingTraits;
> +    using llvm::yaml::IO;
> +
> +    struct Stuff {
> +      ...
> +    };
> +
> +    template <>
> +    struct MappingTraits<Stuff> {
> +      static void mapping(IO &io, Stuff &stuff) {
> +        ...
> +      }
> +
> +      static const bool flow = true;
> +    }
> +
>  
>  Sequence
>  ========
> 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();
> +  }
>  }
>  
>  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;
> @@ -1060,13 +1083,22 @@
>    void newLineCheck();
>    void outputNewLine();
>    void paddedKey(StringRef key);
> -
> -  enum InState { inSeq, inFlowSeq, inMapFirstKey, inMapOtherKey };
> +  void flowKey(StringRef Key);
> +
> +  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,13 @@
>                            bool &UseDefault, void *&) {
>    UseDefault = false;
>    if (Required || !SameAsDefault) {
> -    this->newLineCheck();
> -    this->paddedKey(Key);
> +    auto State = StateStack.back();
> +    if (State == inFlowMapFirstKey || State == inFlowMapOtherKey) {
> +      flowKey(Key);
> +    } else {
> +      this->newLineCheck();
> +      this->paddedKey(Key);
> +    }
>      return true;
>    }
>    return false;
> @@ -438,9 +448,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 +632,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 +661,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;
> @@ -659,6 +687,20 @@
>      output(" ");
>  }
>  
> +void Output::flowKey(StringRef Key) {
> +  if (StateStack.back() == inFlowMapOtherKey)
> +    output(", ");
> +  if (Column > 70) {
> +    output("\n");
> +    for (int I = 0; I < ColumnAtMapFlowStart; ++I)
> +      output(" ");
> +    Column = ColumnAtMapFlowStart;
> +    output("  ");
> +  }
> +  output(Key);
> +  output(": ");
> +}
> +
>  //===----------------------------------------------------------------------===//
>  //  traits for built-in types
>  //===----------------------------------------------------------------------===//
> 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