[YAML] Quote multiline string scalars

[ilya-biryukov]

Summary

Otherwise, the YAML parser breaks when trying to read them back in
'key: multiline_string_value' cases.

This patch fixes a problem when serializing structs which contain multi-line strings.
E.g., if we try to serialize the following struct

{ "key1": "first line\nsecond line",
  "key2": "another string" }`

Before this patch, we got the YAML output that failed to parse:

key1: first line
second line
key2: another string

After the patch, we get:

key1: 'first line
second line'
key2: another string

[ilya-biryukov]

• Fix formatting

[sammccall]

As discussed offline, it'd be nice to assert the actual serialized output in the test. At least in principle we want this to interop with *other* parsers.

include/llvm/Support/YAMLTraits.h

543 ↗

(On Diff #148874)

allowed -> allowed in unquoted strings (for clarity)

548 ↗

(On Diff #148874)

0xa and 0xd are both printable, so single-quoting should be sufficient

550 ↗

(On Diff #148874)

NEL is U+0085, but that won't encode to a 0x85 byte (at least not under UTF-8). So mentioning 0x85 in this switch is a (pre-existing) bug, let's just remove it.

[ilya-biryukov]

• Add deserialized output to tests.

[ilya-biryukov]

• Single-quote strings with \n and \r instead of double-quoting

[sammccall]

Thanks, LG!
comment nit to prove i read it ;)

include/llvm/Support/YAMLTraits.h

546 ↗

(On Diff #149057)

this echoes the code a bit. what about:
LF/CR may delimit values and so require at least single quotes.

[ilya-biryukov]

• Addressed last nit: update the comment

include/llvm/Support/YAMLTraits.h

546 ↗

(On Diff #149057)

Thanks!

[ilya-biryukov]

• Fix a typo

llvm/trunk/include/llvm/Support/YAMLTraits.h

for (unsigned char C : S) {
     // Safe scalar characters.
     case '_':
     case '-':
     case '/':
     case '^':
     case '.':
     case ',':
     case ' ':
-    // TAB (0x9), LF (0xA), CR (0xD) and NEL (0x85) are allowed.
+    // TAB (0x9) is allowed in unquoted strings.
     case 0x9:
+      continue;
+    // LF(0xA) and CR(0xD) may delimit values and so require at least single
+    // quotes.
     case 0xA:
     case 0xD:
-    case 0x85:
+      MaxQuotingNeeded = QuotingType::Single;
       continue;
     // DEL (0x7F) are excluded from the allowed character range.
     case 0x7F:
       return QuotingType::Double;
     default: {
       // C0 control block (0x0 - 0x1F) is excluded from the allowed character
       // range.
       if (C <= 0x1F)

llvm/trunk/unittests/Support/YAMLIOTest.cpp

 //===- unittest/Support/YAMLIOTest.cpp ------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/YAMLTraits.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 using llvm::yaml::Input;
 using llvm::yaml::Output;
 using llvm::yaml::IO;
 using llvm::yaml::MappingTraits;
 using llvm::yaml::MappingNormalization;
 using llvm::yaml::ScalarTraits;
 using llvm::yaml::Hex8;
 using llvm::yaml::Hex16;
 using llvm::yaml::Hex32;
 using llvm::yaml::Hex64;
+using ::testing::StartsWith;
 
 
 
 
 static void suppressErrorMessages(const llvm::SMDiagnostic &, void *) {
 }
 
 
TEST(YAMLIO, TestGivenFilename) {
   auto Buffer = llvm::MemoryBuffer::getMemBuffer("{ x: 42 }", "foo.yaml");
   Input yin(*Buffer, nullptr, testErrorFilename);
   FooBar Value;
   yin >> Value;
 
   EXPECT_TRUE(!!yin.error());
 }
 
+struct WithStringField {
+  std::string str1;
+  std::string str2;
+  std::string str3;
+};
+
+namespace llvm {
+namespace yaml {
+template <> struct MappingTraits<WithStringField> {
+  static void mapping(IO &io, WithStringField &fb) {
+    io.mapRequired("str1", fb.str1);
+    io.mapRequired("str2", fb.str2);
+    io.mapRequired("str3", fb.str3);
+  }
+};
+} // namespace yaml
+} // namespace llvm
+
+TEST(YAMLIO, MultilineStrings) {
+  WithStringField Original;
+  Original.str1 = "a multiline string\nfoobarbaz";
+  Original.str2 = "another one\rfoobarbaz";
+  Original.str3 = "a one-line string";
+
+  std::string Serialized;
+  {
+    llvm::raw_string_ostream OS(Serialized);
+    Output YOut(OS);
+    YOut << Original;
+  }
+  auto Expected = "---\n"
+                  "str1:            'a multiline string\n"
+                  "foobarbaz'\n"
+                  "str2:            'another one\r"
+                  "foobarbaz'\n"
+                  "str3:            a one-line string\n"
+                  "...\n";
+  ASSERT_EQ(Serialized, Expected);
+
+  // Also check it parses back without the errors.
+  WithStringField Deserialized;
+  {
+    Input YIn(Serialized);
+    YIn >> Deserialized;
+    ASSERT_FALSE(YIn.error())
+        << "Parsing error occurred during deserialization. Serialized string:\n"
+        << Serialized;
+  }
+  EXPECT_EQ(Original.str1, Deserialized.str1);
+  EXPECT_EQ(Original.str2, Deserialized.str2);
+  EXPECT_EQ(Original.str3, Deserialized.str3);
+}
+
+TEST(YAMLIO, NoQuotesForTab) {
+  WithStringField WithTab;
+  WithTab.str1 = "aba\tcaba";
+  std::string Serialized;
+  {
+    llvm::raw_string_ostream OS(Serialized);
+    Output YOut(OS);
+    YOut << WithTab;
+  }
+  auto ExpectedPrefix = "---\n"
+                        "str1:            aba\tcaba\n";
+  EXPECT_THAT(Serialized, StartsWith(ExpectedPrefix));
+}
 
 //===----------------------------------------------------------------------===//
 //  Test built-in types
 //===----------------------------------------------------------------------===//
 
 struct BuiltInTypes {
   llvm::StringRef str;
   std::string stdstr;