[PATCH] D92755: [YAML] Support extended spellings when parsing bools.

Sean Silva via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 7 12:52:31 PST 2020


silvas added inline comments.


================
Comment at: llvm/unittests/Support/YAMLParserTest.cpp:356
+TEST(YAMLParser, ParsesBools) {
+  expectCanParseBool("True", true);
+  expectCanParseBool("true", true);
----------------
silvas wrote:
> njames93 wrote:
> > silvas wrote:
> > > 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).
> > I'm a little lost in what you are asking. Comments where exactly?
> 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");
> ```
(I think case-insensitive sort might be better honestly)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92755



More information about the llvm-commits mailing list