[yaml][clang-tidy] Fix multiline YAML serialization

Summary:
New line duplication logic introduced in https://reviews.llvm.org/D63482
has two issues: (1) there is no logic that removes duplicate newlines
when clang-apply-replacment reads YAML and (2) in general such logic
should be applied to all strings and should happen on string
serialization level instead in YAML parser.

This diff changes multiline strings quotation from single quote `'` to
double `"`. It solves problems with internal newlines because now they are
escaped. Also double quotation solves the problem with leading whitespace after
newline. In case of single quotation YAML parsers should remove leading
whitespace according to specification. In case of double quotation these
leading are internal space and they are preserved. There is no way to
instruct YAML parsers to preserve leading whitespaces after newline so
double quotation is the only viable option that solves all problems at
once.

Test Plan: check-all

Reviewers: gribozavr, mgehre, yvvan

Subscribers: xazax.hun, hiraditya, cfe-commits, llvm-commits

Tags: #clang-tools-extra, #clang, #llvm

Differential Revision: https://reviews.llvm.org/D80301

clang/include/clang/Tooling/ReplacementsYaml.h

@@ -35,13 +35,7 @@ template <> struct MappingTraits<clang::tooling::Replacement> {
 
     NormalizedReplacement(const IO &, const clang::tooling::Replacement &R)
         : FilePath(R.getFilePath()), Offset(R.getOffset()),
-          Length(R.getLength()), ReplacementText(R.getReplacementText()) {
-      size_t lineBreakPos = ReplacementText.find('\n');
-      while (lineBreakPos != std::string::npos) {
-        ReplacementText.replace(lineBreakPos, 1, "\n\n");
-        lineBreakPos = ReplacementText.find('\n', lineBreakPos + 2);
-      }
-    }
+          Length(R.getLength()), ReplacementText(R.getReplacementText()) {}
 
     clang::tooling::Replacement denormalize(const IO &) {
       return clang::tooling::Replacement(FilePath, Offset, Length,

clang/unittests/Tooling/ReplacementsYamlTest.cpp

@@ -65,7 +65,7 @@ TEST(ReplacementsYamlTest, serializesNewLines) {
                "  - FilePath:        '/path/to/file1.h'\n"
                "    Offset:          0\n"
                "    Length:          0\n"
-               "    ReplacementText: '#include <utility>\n\n'\n"
+               "    ReplacementText: \"#include <utility>\\n\"\n"
                "...\n",
                YamlContentStream.str().c_str());
 }

llvm/include/llvm/Support/YAMLTraits.h

@@ -649,24 +649,25 @@ inline bool isBool(StringRef S) {
 inline QuotingType needsQuotes(StringRef S) {
   if (S.empty())
     return QuotingType::Single;
+
+  QuotingType MaxQuotingNeeded = QuotingType::None;
   if (isSpace(static_cast<unsigned char>(S.front())) ||
       isSpace(static_cast<unsigned char>(S.back())))
-    return QuotingType::Single;
+    MaxQuotingNeeded = QuotingType::Single;
   if (isNull(S))
-    return QuotingType::Single;
+    MaxQuotingNeeded = QuotingType::Single;
   if (isBool(S))
-    return QuotingType::Single;
+    MaxQuotingNeeded = QuotingType::Single;
   if (isNumeric(S))
-    return QuotingType::Single;
+    MaxQuotingNeeded = QuotingType::Single;
 
   // 7.3.3 Plain Style
   // Plain scalars must not begin with most indicators, as this would cause
   // ambiguity with other YAML constructs.
   static constexpr char Indicators[] = R"(-?:\,[]{}#&*!|>'"%@`)";
   if (S.find_first_of(Indicators) == 0)
-    return QuotingType::Single;
+    MaxQuotingNeeded = QuotingType::Single;
 
-  QuotingType MaxQuotingNeeded = QuotingType::None;
   for (unsigned char C : S) {
     // Alphanum is safe.
     if (isAlnum(C))
@@ -684,11 +685,11 @@ inline QuotingType needsQuotes(StringRef S) {
     case 0x9:
       continue;
     // LF(0xA) and CR(0xD) may delimit values and so require at least single
-    // quotes.
+    // quotes. LLVM YAML parser cannot handle single quoted multiline so use
+    // double quoting to produce valid YAML.
     case 0xA:
     case 0xD:
-      MaxQuotingNeeded = QuotingType::Single;
-      continue;
+      return QuotingType::Double;
     // DEL (0x7F) are excluded from the allowed character range.
     case 0x7F:
       return QuotingType::Double;

llvm/lib/Support/YAMLTraits.cpp

@@ -878,12 +878,12 @@ StringRef ScalarTraits<StringRef>::input(StringRef Scalar, void *,
 }
 
 void ScalarTraits<std::string>::output(const std::string &Val, void *,
-                                     raw_ostream &Out) {
+                                       raw_ostream &Out) {
   Out << Val;
 }
 
 StringRef ScalarTraits<std::string>::input(StringRef Scalar, void *,
-                                         std::string &Val) {
+                                           std::string &Val) {
   Val = Scalar.str();
   return StringRef();
 }

llvm/test/Transforms/LowerMatrixIntrinsics/remarks-shared-subtrees.ll

@@ -18,8 +18,7 @@
 ; YAML-NEXT:    - String:          ' loads, '
 ; YAML-NEXT:    - NumComputeOps:   '0'
 ; YAML-NEXT:    - String:          ' compute ops'
-; YAML-NEXT:    - String:          ',
-; YAML-NEXT:  additionally '
+; YAML-NEXT:    - String:          ",\nadditionally "
 ; YAML-NEXT:    - NumStores:       '0'
 ; YAML-NEXT:    - String:          ' stores, '
 ; YAML-NEXT:    - NumLoads:        '4'
@@ -47,8 +46,7 @@
 ; YAML-NEXT:    - String:          ' loads, '
 ; YAML-NEXT:    - NumComputeOps:   '120'
 ; YAML-NEXT:    - String:          ' compute ops'
-; YAML-NEXT:    - String:          ',
-; YAML-NEXT:  additionally '
+; YAML-NEXT:    - String:          ",\nadditionally "
 ; YAML-NEXT:    - NumStores:       '0'
 ; YAML-NEXT:    - String:          ' stores, '
 ; YAML-NEXT:    - NumLoads:        '4'

llvm/unittests/Support/YAMLIOTest.cpp

@@ -285,10 +285,8 @@ TEST(YAMLIO, MultilineStrings) {
     YOut << Original;
   }
   auto Expected = "---\n"
-                  "str1:            'a multiline string\n"
-                  "foobarbaz'\n"
-                  "str2:            'another one\r"
-                  "foobarbaz'\n"
+                  "str1:            \"a multiline string\\nfoobarbaz\"\n"
+                  "str2:            \"another one\\rfoobarbaz\"\n"
                   "str3:            a one-line string\n"
                   "...\n";
   ASSERT_EQ(Serialized, Expected);