[YAML] Support extended spellings when parsing bools.

[njames93]

Summary

Support all the spellings of boolean datatypes according to https://yaml.org/type/bool.html

[silvas]

The spec link doesn't seem to support AllowNumeric. Why do we have that feature?

[njames93]

In D92755#2437578, @silvas wrote:

The spec link doesn't seem to support AllowNumeric. Why do we have that feature?

Its disabled by default, It was added for partial support of a child of this, but thinking about it, it may be better to extract that behaviour out.

[njames93]

Removed AllowNumeric support

[silvas]

Thanks. This LGTM.

llvm/unittests/Support/YAMLParserTest.cpp

356

Nit: I find these a little hard to scan. Can we separate the "true" and "false" cases with comments, and then sort within each one (it's not obvious to me how the current ones are sorted).

[njames93]

Also I may need to investigate the pre-merge, some tests were failing, some looked like they involved yaml and are related to this. Keys called N were missing because the parser was treating them as a bool it seems. Will let it build again to be sure it is this patch causing the issue.

llvm/unittests/Support/YAMLParserTest.cpp

356

I'm a little lost in what you are asking. Comments where exactly?

[silvas]

llvm/unittests/Support/YAMLParserTest.cpp

356

Something like this. Sorting is whatever vim's :sort gave me.

// Test true values.
expectCanParseBool("ON", true);
expectCanParseBool("On", true);
expectCanParseBool("TRUE", true);
expectCanParseBool("True", true);
expectCanParseBool("Y", true);
expectCanParseBool("YES", true);
expectCanParseBool("Yes", true);
expectCanParseBool("on", true);
expectCanParseBool("true", true);
expectCanParseBool("y", true);
expectCanParseBool("yes", true);
expectCannotParseBool("1");

// Test false values.
expectCanParseBool("FALSE", false);
expectCanParseBool("False", false);
expectCanParseBool("N", false);
expectCanParseBool("NO", false);
expectCanParseBool("No", false);
expectCanParseBool("OFF", false);
expectCanParseBool("Off", false);
expectCanParseBool("false", false);
expectCanParseBool("n", false);
expectCanParseBool("no", false);
expectCanParseBool("off", false);
expectCannotParseBool("0");

356

(I think case-insensitive sort might be better honestly)

[njames93]

llvm/include/llvm/Support/YAMLTraits.h

639

Ahh so this is causing the bool value set to be quoted, even the value isn't being used as a bool. Resulting in test case failures.

[njames93]

Fix other tests failing(hopefully).
Address order of tests and comments.

llvm/include/llvm/Support/YAMLParser.h

 bool scanTokens(StringRef Input);
 
 /// Escape \a Input for a double quoted scalar; if \p EscapePrintable
 /// is true, all UTF8 sequences will be escaped, if \p EscapePrintable is
 /// false, those UTF8 sequences encoding printable unicode scalars will not be
 /// escaped, but emitted verbatim.
 std::string escape(StringRef Input, bool EscapePrintable = true);
 
+/// Parse \p S as a bool according to https://yaml.org/type/bool.html.
+/// If \p AllowNumeric is set: '0' will be parsed as `false`, any other integer
+/// will be parsed as `true`, Otherwise All integers will fail to parse.
+llvm::Optional<bool> parseBool(StringRef S, bool AllowNumeric = false);
+
 /// This class represents a YAML stream potentially containing multiple
 ///        documents.
 class Stream {
 public:
   /// This keeps a reference to the string referenced by \p Input.
   Stream(StringRef Input, SourceMgr &, bool ShowColors = true,
          std::error_code *EC = nullptr);
 

llvm/include/llvm/Support/YAMLTraits.h

inline bool isNumeric(StringRef S) {
   return skipDigits(S).empty();
 }
 
 inline bool isNull(StringRef S) {
   return S.equals("null") || S.equals("Null") || S.equals("NULL") ||
          S.equals("~");
 }
 
-inline bool isBool(StringRef S) {
-  return S.equals("true") || S.equals("True") || S.equals("TRUE") ||
-         S.equals("false") || S.equals("False") || S.equals("FALSE");
-}
+inline bool isBool(StringRef S) { return parseBool(S).hasValue(); }
 
 // 5.1. Character Set
 // The allowed character range explicitly excludes the C0 control block #x0-#x1F
 // (except for TAB #x9, LF #xA, and CR #xD which are allowed), DEL #x7F, the C1
 // control block #x80-#x9F (except for NEL #x85 which is allowed), the surrogate
 // block #xD800-#xDFFF, #xFFFE, and #xFFFF.
 inline QuotingType needsQuotes(StringRef S) {
   if (S.empty())

llvm/lib/Support/YAMLParser.cpp

else if ((unsigned char)*i < 0x20) { // Control characters not handled above.
       }
       i += UnicodeScalarValue.second - 1;
     } else
       EscapedInput.push_back(*i);
   }
   return EscapedInput;
 }
 
+llvm::Optional<bool> yaml::parseBool(StringRef S, bool AllowNumeric) {
+  if (AllowNumeric) {
+    long long Number;
+    if (!S.getAsInteger(10, Number))
+      return Number != 0;
+  }
+  switch (S.size()) {
+  case 1:
+    switch (S.front()) {
+    case 'y':
+    case 'Y':
+      return true;
+    case 'n':
+    case 'N':
+      return false;
+    default:
+      return None;
+    }
+  case 2:
+    switch (S.front()) {
+    case 'O':
+      if (S[1] == 'N') // ON
+        return true;
+      LLVM_FALLTHROUGH;
+    case 'o':
+      if (S[1] == 'n') //[Oo]n
+        return true;
+      return None;
+    case 'N':
+      if (S[1] == 'O') // NO
+        return false;
+      LLVM_FALLTHROUGH;
+    case 'n':
+      if (S[1] == 'o') //[Nn]o
+        return false;
+      return None;
+    default:
+      return None;
+    }
+  case 3:
+    switch (S.front()) {
+    case 'O':
+      if (S.drop_front() == "FF") // OFF
+        return false;
+      LLVM_FALLTHROUGH;
+    case 'o':
+      if (S.drop_front() == "ff") //[Oo]ff
+        return false;
+      return None;
+    case 'Y':
+      if (S.drop_front() == "ES") // YES
+        return true;
+      LLVM_FALLTHROUGH;
+    case 'y':
+      if (S.drop_front() == "es") //[Yy]es
+        return true;
+      return None;
+    default:
+      return None;
+    }
+  case 4:
+    switch (S.front()) {
+    case 'T':
+      if (S.drop_front() == "RUE") // TRUE
+        return true;
+      LLVM_FALLTHROUGH;
+    case 't':
+      if (S.drop_front() == "rue") //[Tt]rue
+        return true;
+      return None;
+    default:
+      return None;
+    }
+  case 5:
+    switch (S.front()) {
+    case 'F':
+      if (S.drop_front() == "ALSE") // FALSE
+        return false;
+      LLVM_FALLTHROUGH;
+    case 'f':
+      if (S.drop_front() == "alse") //[Ff]alse
+        return false;
+      return None;
+    default:
+      return None;
+    }
+  default:
+    return None;
+  }
+}
+
 Scanner::Scanner(StringRef Input, SourceMgr &sm, bool ShowColors,
                  std::error_code *EC)
     : SM(sm), ShowColors(ShowColors), EC(EC) {
   init(MemoryBufferRef(Input, "YAML"));
 }
 
 Scanner::Scanner(MemoryBufferRef Buffer, SourceMgr &SM_, bool ShowColors,
                  std::error_code *EC)

llvm/lib/Support/YAMLTraits.cpp

 //  traits for built-in types
 //===----------------------------------------------------------------------===//
 
 void ScalarTraits<bool>::output(const bool &Val, void *, raw_ostream &Out) {
   Out << (Val ? "true" : "false");
 }
 
 StringRef ScalarTraits<bool>::input(StringRef Scalar, void *, bool &Val) {
-  if (Scalar.equals("true")) {
-    Val = true;
-    return StringRef();
-  } else if (Scalar.equals("false")) {
-    Val = false;
+  if (llvm::Optional<bool> Parsed = parseBool(Scalar)) {
+    Val = *Parsed;
     return StringRef();
   }
   return "invalid boolean";
 }
 
 void ScalarTraits<StringRef>::output(const StringRef &Val, void *,
                                      raw_ostream &Out) {
   Out << Val;

llvm/unittests/Support/YAMLParserTest.cpp

TEST(YAMLParser, FlowSequenceTokensOutsideFlowSequence) {
 
   for (auto &Str : FlowSequenceStrs) {
     yaml::Stream Stream(Str, SM);
     yaml::Document &Doc = *Stream.begin();
     EXPECT_FALSE(Doc.skip());
   }
 }
 
+static void expectCanParseBool(StringRef S, bool Expected,
+                               bool AllowNumeric = false) {
+  llvm::Optional<bool> Parsed = yaml::parseBool(S, AllowNumeric);
+  EXPECT_TRUE(Parsed.hasValue());
+  EXPECT_EQ(*Parsed, Expected);
+}
+
+static void expectCannotParseBool(StringRef S, bool AllowNumeric = false) {
+  EXPECT_FALSE(yaml::parseBool(S, AllowNumeric).hasValue());
+}
+
+TEST(YAMLParser, ParsesBools) {
+  expectCanParseBool("True", true);
+  expectCanParseBool("true", true);
+  expectCanParseBool("TRUE", true);
+  expectCanParseBool("On", true);
+  expectCanParseBool("on", true);
+  expectCanParseBool("ON", true);
+  expectCanParseBool("Yes", true);
+  expectCanParseBool("yes", true);
+  expectCanParseBool("YES", true);
+  expectCanParseBool("y", true);
+  expectCanParseBool("Y", true);
+  expectCanParseBool("1", true, /*AllowNumeric=*/true);
+  expectCannotParseBool("1", /*AllowNumeric=*/false);
+  expectCanParseBool("False", false);
+  expectCanParseBool("false", false);
+  expectCanParseBool("FALSE", false);
+  expectCanParseBool("Off", false);
+  expectCanParseBool("off", false);
+  expectCanParseBool("OFF", false);
+  expectCanParseBool("No", false);
+  expectCanParseBool("no", false);
+  expectCanParseBool("NO", false);
+  expectCanParseBool("n", false);
+  expectCanParseBool("N", false);
+  expectCanParseBool("0", false, /*AllowNumeric=*/true);
+  expectCannotParseBool("0", /*AllowNumeric=*/false);
+}
+
 } // end namespace llvm