[PATCH] D42362: [YAML] Plain scalars can not begin with most indicators.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: llvm-commits, ioeric, ilya-biryukov.

Discovered when clangd loads YAML symbols, some symbol documentations
start with indicators (e.g. "-"), but YAML prints them as plain scalars
(no quotes), which make the YAML parser fail to parse.

For these kind of strings, we need quotes.


Repository:
  rL LLVM

https://reviews.llvm.org/D42362

Files:
  include/llvm/Support/YAMLTraits.h
  unittests/Support/YAMLIOTest.cpp


Index: unittests/Support/YAMLIOTest.cpp
===================================================================
--- unittests/Support/YAMLIOTest.cpp
+++ unittests/Support/YAMLIOTest.cpp
@@ -533,6 +533,7 @@
   std::string stdstr9;
   std::string stdstr10;
   std::string stdstr11;
+  std::string stdstr12;
 };
 
 namespace llvm {
@@ -562,6 +563,7 @@
       io.mapRequired("stdstr9",   st.stdstr9);
       io.mapRequired("stdstr10",  st.stdstr10);
       io.mapRequired("stdstr11",  st.stdstr11);
+      io.mapRequired("stdstr12",  st.stdstr12);
     }
   };
 }
@@ -593,6 +595,7 @@
     map.stdstr9 = "~";
     map.stdstr10 = "0.2e20";
     map.stdstr11 = "0x30";
+    map.stdstr12 = "- match";
 
     llvm::raw_string_ostream ostr(intermediate);
     Output yout(ostr);
@@ -611,6 +614,7 @@
   EXPECT_NE(llvm::StringRef::npos, flowOut.find("'~'\n"));
   EXPECT_NE(llvm::StringRef::npos, flowOut.find("'0.2e20'\n"));
   EXPECT_NE(llvm::StringRef::npos, flowOut.find("'0x30'\n"));
+  EXPECT_NE(llvm::StringRef::npos, flowOut.find("'- match'\n"));
   EXPECT_NE(std::string::npos, flowOut.find("'''eee"));
   EXPECT_NE(std::string::npos, flowOut.find("'\"fff'"));
   EXPECT_NE(std::string::npos, flowOut.find("'`ggg'"));
Index: include/llvm/Support/YAMLTraits.h
===================================================================
--- include/llvm/Support/YAMLTraits.h
+++ include/llvm/Support/YAMLTraits.h
@@ -511,15 +511,23 @@
     return QuotingType::Single;
   if (isspace(S.front()) || isspace(S.back()))
     return QuotingType::Single;
-  if (S.front() == ',')
-    return QuotingType::Single;
   if (isNull(S))
     return QuotingType::Single;
   if (isBool(S))
     return QuotingType::Single;
   if (isNumeric(S))
     return QuotingType::Single;
 
+  // 7.3.3 Plain Style
+  // Plain scalars must not begin with most indicators, as this would cause
+  // ambiguity with other YAML constructs.
+  // FIXME: However, the “:”, “?” and “-” indicators may be used as the first
+  // character if followed by a non-space “safe” character, as this causes no
+  // ambiguity.
+  static constexpr char Indicators[] = R"(-?:\,[]{}#&*!|>'"%@`)";
+  if (S.find_first_of(Indicators) == 0)
+    return QuotingType::Single;
+
   QuotingType MaxQuotingNeeded = QuotingType::None;
   for (unsigned char C : S) {
     // Alphanum is safe.



Index: unittests/Support/YAMLIOTest.cpp
===================================================================
--- unittests/Support/YAMLIOTest.cpp
+++ unittests/Support/YAMLIOTest.cpp
@@ -533,6 +533,7 @@
   std::string stdstr9;
   std::string stdstr10;
   std::string stdstr11;
+  std::string stdstr12;
 };
 
 namespace llvm {
@@ -562,6 +563,7 @@
       io.mapRequired("stdstr9",   st.stdstr9);
       io.mapRequired("stdstr10",  st.stdstr10);
       io.mapRequired("stdstr11",  st.stdstr11);
+      io.mapRequired("stdstr12",  st.stdstr12);
     }
   };
 }
@@ -593,6 +595,7 @@
     map.stdstr9 = "~";
     map.stdstr10 = "0.2e20";
     map.stdstr11 = "0x30";
+    map.stdstr12 = "- match";
 
     llvm::raw_string_ostream ostr(intermediate);
     Output yout(ostr);
@@ -611,6 +614,7 @@
   EXPECT_NE(llvm::StringRef::npos, flowOut.find("'~'\n"));
   EXPECT_NE(llvm::StringRef::npos, flowOut.find("'0.2e20'\n"));
   EXPECT_NE(llvm::StringRef::npos, flowOut.find("'0x30'\n"));
+  EXPECT_NE(llvm::StringRef::npos, flowOut.find("'- match'\n"));
   EXPECT_NE(std::string::npos, flowOut.find("'''eee"));
   EXPECT_NE(std::string::npos, flowOut.find("'\"fff'"));
   EXPECT_NE(std::string::npos, flowOut.find("'`ggg'"));
Index: include/llvm/Support/YAMLTraits.h
===================================================================
--- include/llvm/Support/YAMLTraits.h
+++ include/llvm/Support/YAMLTraits.h
@@ -511,15 +511,23 @@
     return QuotingType::Single;
   if (isspace(S.front()) || isspace(S.back()))
     return QuotingType::Single;
-  if (S.front() == ',')
-    return QuotingType::Single;
   if (isNull(S))
     return QuotingType::Single;
   if (isBool(S))
     return QuotingType::Single;
   if (isNumeric(S))
     return QuotingType::Single;
 
+  // 7.3.3 Plain Style
+  // Plain scalars must not begin with most indicators, as this would cause
+  // ambiguity with other YAML constructs.
+  // FIXME: However, the “:”, “?” and “-” indicators may be used as the first
+  // character if followed by a non-space “safe” character, as this causes no
+  // ambiguity.
+  static constexpr char Indicators[] = R"(-?:\,[]{}#&*!|>'"%@`)";
+  if (S.find_first_of(Indicators) == 0)
+    return QuotingType::Single;
+
   QuotingType MaxQuotingNeeded = QuotingType::None;
   for (unsigned char C : S) {
     // Alphanum is safe.

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits