DmitryPolukhin updated this revision to Diff 272952.
DmitryPolukhin added a comment.

Use double quotation for multiline strings. 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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80301/new/

https://reviews.llvm.org/D80301

Files:
  clang/include/clang/Tooling/ReplacementsYaml.h
  clang/unittests/Tooling/ReplacementsYamlTest.cpp
  llvm/include/llvm/Support/YAMLTraits.h
  llvm/lib/Support/YAMLTraits.cpp
  llvm/unittests/Support/YAMLIOTest.cpp

Index: llvm/unittests/Support/YAMLIOTest.cpp
===================================================================
--- llvm/unittests/Support/YAMLIOTest.cpp
+++ llvm/unittests/Support/YAMLIOTest.cpp
@@ -285,10 +285,8 @@
     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);
Index: llvm/lib/Support/YAMLTraits.cpp
===================================================================
--- llvm/lib/Support/YAMLTraits.cpp
+++ llvm/lib/Support/YAMLTraits.cpp
@@ -878,12 +878,12 @@
 }
 
 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();
 }
Index: llvm/include/llvm/Support/YAMLTraits.h
===================================================================
--- llvm/include/llvm/Support/YAMLTraits.h
+++ llvm/include/llvm/Support/YAMLTraits.h
@@ -649,24 +649,25 @@
 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 @@
     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;
Index: clang/unittests/Tooling/ReplacementsYamlTest.cpp
===================================================================
--- clang/unittests/Tooling/ReplacementsYamlTest.cpp
+++ clang/unittests/Tooling/ReplacementsYamlTest.cpp
@@ -65,7 +65,7 @@
                "  - 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());
 }
Index: clang/include/clang/Tooling/ReplacementsYaml.h
===================================================================
--- clang/include/clang/Tooling/ReplacementsYaml.h
+++ clang/include/clang/Tooling/ReplacementsYaml.h
@@ -35,13 +35,7 @@
 
     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,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to