[support] Some improvements to StringSwitch

[zturner]

Summary

This patch does a number of things:

1. Adds case-insensitive versions of all case functions. This means CaseLower, StartsWithLower, EndsWithLower, and CasesLower.
2. Converts the Cases function to use variadic templates. Thanks to rsmith@ for the magic incantation to make this work.
3. Adds an alternative to Default() that returns an llvm::Expected<T>. This is useful when you Default is insufficient to determine that a switch fell off the end.
4. Adds unit tests for the new case-insensitive versions of the function, as well as the old ones (there were previously 0 unit tests for StringSwitch).

[zturner]

Sadly, the variadic template idea died once I learned that a temporary is being created in the variadic function, and chained calls dont' work as a result. I will go back to using multiple methods :(

[zturner]

Removed variadic template.

[zturner]

Rebase against ToT

[chandlerc]

Could you split this appart? I think it will be much easier to review.

Suggested split and ordering:

1. Add basic unit tests. Should be super easy patch.
2. Switch to variadic templates with associated magic. Should have a nice simple unit test that showcases what is going on with this patch. And the unit tests from #1 should clearly show that it doesn't break things.
3. Add the case insensitive case methods.
4. Add the variant of Default. However, I wouldn't use Error and Expected here. Those seem both heavyweight and indicative of an *error* as opposed to just detecting whether or not the switch fell off the end. I feel like Optional would be a better fit?

Clearly #3 and #4 can be done in parallel. Happy to discuss any high level issues here to avoid re-writing stuff again and again in rebased patches just to change the underlying design.

[zturner]

Sadly, the variadic template approach died after Richard and I found out that it couldn't be made to work. So all we've got now is the addition of Lower versions of the functions. I just realized that the test file didn't make it to the review, so I'll get that uploaded and change to using Optional. Do you think it's still worth splitting if we're not doing the variadic templates?

[zturner]

Here's the updated patch, including Chandler's suggestion of using Optional instead of Error. I thought about this some more and I don't see the value-add associated with splitting this apart. Primarily because I abandoned the variadic templates idea. As such, the Lower versions of the functions must go in at the same time as the Lower versions of the tests, and those two things constitue 90% of the entire patch (the rest are just case-sensitive versions of the tests for existing functions which have not changed in this patch).

Let me know if you disagree.

[ruiu]

I wonder if we need OrNone. Isn't it equivalent to .Default(None)?

[zturner]

In D24686#559270, @ruiu wrote:

I wonder if we need OrNone. Isn't it equivalent to .Default(None)?

It works if we add an additional overload of Default() like this:

Optional<R> Default(llvm::NoneType N) const {
  if (Result)
    return *Result;
  return None;
}

If people prefer that I can go with that instead of OrNone

[chandlerc]

In D24686#559231, @zturner wrote:

Here's the updated patch, including Chandler's suggestion of using Optional instead of Error. I thought about this some more and I don't see the value-add associated with splitting this apart. Primarily because I abandoned the variadic templates idea. As such, the Lower versions of the functions must go in at the same time as the Lower versions of the tests, and those two things constitue 90% of the entire patch (the rest are just case-sensitive versions of the tests for existing functions which have not changed in this patch).

Let me know if you disagree.

I kind of do... It makes the code review and any potential reverts much more clear IMO.

[zturner]

Unit tests for the case-sensitive versions were removed and submitted separately. The Optional variant of the function is removed and will be submitted later. This patch only contains case-insensitive versions of the functions with associated unit tests.

[ruiu]

LGTM

[chandlerc]

LGTM as well. I like the FooLower pattern. While somewhat verbose and repetitive, it seems much more clear and obvious to the reader. It is also much simpler.

include/llvm/ADT/StringSwitch.h

 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Compiler.h"
+#include "llvm/Support/Error.h"
 #include <cassert>
 #include <cstring>
 

 
   ~StringSwitch() = default;
 
+  // Case-sensitive case matchers
   template<unsigned N>
   LLVM_ATTRIBUTE_ALWAYS_INLINE
   StringSwitch& Case(const char (&S)[N], const T& Value) {

     return *this;
   }
 
+  // Case-insensitive case matchers.
+  template <unsigned N>
+  LLVM_ATTRIBUTE_ALWAYS_INLINE StringSwitch &CaseLower(const char (&S)[N],
+                                                       const T &Value) {
+    if (!Result && Str.equals_lower(StringRef(S, N - 1)))
+      Result = &Value;
+
+    return *this;
+  }
+
+  template <unsigned N>
+  LLVM_ATTRIBUTE_ALWAYS_INLINE StringSwitch &EndsWithLower(const char (&S)[N],
+                                                           const T &Value) {
+    if (!Result && Str.endswith_lower(StringRef(S, N - 1)))
+      Result = &Value;
+
+    return *this;
+  }
+
+  template <unsigned N>
+  LLVM_ATTRIBUTE_ALWAYS_INLINE StringSwitch &StartsWithLower(const char (&S)[N],
+                                                             const T &Value) {
+    if (!Result && Str.startswith_lower(StringRef(S, N - 1)))
+      Result = &Value;
+
+    return *this;
+  }
+  template <unsigned N0, unsigned N1>
+  LLVM_ATTRIBUTE_ALWAYS_INLINE StringSwitch &
+  CasesLower(const char (&S0)[N0], const char (&S1)[N1], const T &Value) {
+    return CaseLower(S0, Value).CaseLower(S1, Value);
+  }
+
+  template <unsigned N0, unsigned N1, unsigned N2>
+  LLVM_ATTRIBUTE_ALWAYS_INLINE StringSwitch &
+  CasesLower(const char (&S0)[N0], const char (&S1)[N1], const char (&S2)[N2],
+             const T &Value) {
+    return CaseLower(S0, Value).CasesLower(S1, S2, Value);
+  }
+
+  template <unsigned N0, unsigned N1, unsigned N2, unsigned N3>
+  LLVM_ATTRIBUTE_ALWAYS_INLINE StringSwitch &
+  CasesLower(const char (&S0)[N0], const char (&S1)[N1], const char (&S2)[N2],
+             const char (&S3)[N3], const T &Value) {
+    return CaseLower(S0, Value).CasesLower(S1, S2, S3, Value);
+  }
+
+  template <unsigned N0, unsigned N1, unsigned N2, unsigned N3, unsigned N4>
+  LLVM_ATTRIBUTE_ALWAYS_INLINE StringSwitch &
+  CasesLower(const char (&S0)[N0], const char (&S1)[N1], const char (&S2)[N2],
+             const char (&S3)[N3], const char (&S4)[N4], const T &Value) {
+    return CaseLower(S0, Value).CasesLower(S1, S2, S3, S4, Value);
+  }
+
   LLVM_ATTRIBUTE_ALWAYS_INLINE
-  R Default(const T& Value) const {
+  R Default(const T &Value) const {
     if (Result)
       return *Result;
 

   }
 
   LLVM_ATTRIBUTE_ALWAYS_INLINE
+  Expected<R> Expect() const {
+    if (Result)
+      return *Result;
+    return make_error<StringError>("Value did not occur in StringSwitch",
+                                   inconvertibleErrorCode());
+  }
+
+  LLVM_ATTRIBUTE_ALWAYS_INLINE
   operator R() const {
     assert(Result && "Fell off the end of a string-switch");
     return *Result;

unittests/ADT/CMakeLists.txt

   SparseSetTest.cpp
   StringMapTest.cpp
   StringRefTest.cpp
+  StringSwitchTest.cpp
   TinyPtrVectorTest.cpp
   TripleTest.cpp
   TwineTest.cpp

unittests/ADT/StringSwitchTest.cpp

+//===- llvm/unittest/ADT/StringSwitchTest.cpp - StringSwitch unit tests ---===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/StringSwitch.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+#define EXPECT_EXPECTED(Expected, Actual)                                      \
+  {                                                                            \
+    auto Result = (Actual);                                                    \
+    auto E = Result.takeError();                                               \
+    EXPECT_FALSE(static_cast<bool>(E));                                        \
+    if (E) {                                                                   \
+      consumeError(std::move(E));                                              \
+    } else {                                                                   \
+      EXPECT_EQ(Expected, *Result);                                            \
+    }                                                                          \
+  }
+
+#define EXPECT_UNEXPECTED(Exp)                                                 \
+  {                                                                            \
+    auto E = Exp.takeError();                                                  \
+    EXPECT_TRUE(static_cast<bool>(E));                                         \
+    if (E) {                                                                   \
+      consumeError(std::move(E));                                              \
+    }                                                                          \
+  }
+
+TEST(StringSwitchTest, CaseSensitiveCase) {
+  auto Translate = [](StringRef S) {
+    return llvm::StringSwitch<int>(S)
+        .Case("0", 0)
+        .Case("1", 1)
+        .Case("2", 2)
+        .Case("3", 3)
+        .Case("4", 4)
+        .Case("5", 5)
+        .Case("6", 6)
+        .Case("7", 7)
+        .Case("8", 8)
+        .Case("9", 9)
+        .Case("A", 10)
+        .Case("B", 11)
+        .Case("C", 12)
+        .Case("D", 13)
+        .Case("E", 14)
+        .Case("F", 15)
+        .Default(-1);
+  };
+  EXPECT_EQ(1, Translate("1"));
+  EXPECT_EQ(2, Translate("2"));
+  EXPECT_EQ(11, Translate("B"));
+  EXPECT_EQ(-1, Translate("b"));
+  EXPECT_EQ(-1, Translate(""));
+  EXPECT_EQ(-1, Translate("Test"));
+}
+
+TEST(StringSwitchTest, CaseInsensitiveCase) {
+  auto Translate = [](StringRef S) {
+    return llvm::StringSwitch<int>(S)
+        .Case("0", 0)
+        .Case("1", 1)
+        .Case("2", 2)
+        .Case("3", 3)
+        .Case("4", 4)
+        .Case("5", 5)
+        .Case("6", 6)
+        .Case("7", 7)
+        .Case("8", 8)
+        .Case("9", 9)
+        .CaseLower("A", 10)
+        .CaseLower("B", 11)
+        .CaseLower("C", 12)
+        .CaseLower("D", 13)
+        .CaseLower("E", 14)
+        .CaseLower("F", 15)
+        .Default(-1);
+  };
+  EXPECT_EQ(1, Translate("1"));
+  EXPECT_EQ(2, Translate("2"));
+  EXPECT_EQ(11, Translate("B"));
+  EXPECT_EQ(11, Translate("b"));
+
+  EXPECT_EQ(-1, Translate(""));
+  EXPECT_EQ(-1, Translate("Test"));
+}
+
+TEST(StringSwitchTest, CaseSensitiveStartsWith) {
+  auto Translate = [](StringRef S) {
+    return llvm::StringSwitch<std::function<int(int, int)>>(S)
+        .StartsWith("add", [](int X, int Y) { return X + Y; })
+        .StartsWith("sub", [](int X, int Y) { return X - Y; })
+        .StartsWith("mul", [](int X, int Y) { return X * Y; })
+        .StartsWith("div", [](int X, int Y) { return X / Y; })
+        .Default([](int X, int Y) { return 0; });
+  };
+
+  EXPECT_EQ(15, Translate("adder")(10, 5));
+  EXPECT_EQ(5, Translate("subtracter")(10, 5));
+  EXPECT_EQ(50, Translate("multiplier")(10, 5));
+  EXPECT_EQ(2, Translate("divider")(10, 5));
+
+  EXPECT_EQ(0, Translate("nothing")(10, 5));
+  EXPECT_EQ(0, Translate("ADDER")(10, 5));
+}
+
+TEST(StringSwitchTest, CaseInensitiveStartsWith) {
+  auto Translate = [](StringRef S) {
+    return llvm::StringSwitch<std::function<int(int, int)>>(S)
+        .StartsWithLower("add", [](int X, int Y) { return X + Y; })
+        .StartsWithLower("sub", [](int X, int Y) { return X - Y; })
+        .StartsWithLower("mul", [](int X, int Y) { return X * Y; })
+        .StartsWithLower("div", [](int X, int Y) { return X / Y; })
+        .Default([](int X, int Y) { return 0; });
+  };
+
+  EXPECT_EQ(15, Translate("adder")(10, 5));
+  EXPECT_EQ(5, Translate("subtracter")(10, 5));
+  EXPECT_EQ(50, Translate("multiplier")(10, 5));
+  EXPECT_EQ(2, Translate("divider")(10, 5));
+
+  EXPECT_EQ(15, Translate("AdDeR")(10, 5));
+  EXPECT_EQ(5, Translate("SuBtRaCtEr")(10, 5));
+  EXPECT_EQ(50, Translate("MuLtIpLiEr")(10, 5));
+  EXPECT_EQ(2, Translate("DiViDeR")(10, 5));
+
+  EXPECT_EQ(0, Translate("nothing")(10, 5));
+}
+
+TEST(StringSwitchTest, CaseSensitiveEndsWith) {
+  enum class Suffix { Possible, PastTense, Process, InProgressAction, Unknown };
+
+  auto Translate = [](StringRef S) {
+    return llvm::StringSwitch<Suffix>(S)
+        .EndsWith("able", Suffix::Possible)
+        .EndsWith("ed", Suffix::PastTense)
+        .EndsWith("ation", Suffix::Process)
+        .EndsWith("ing", Suffix::InProgressAction)
+        .Default(Suffix::Unknown);
+  };
+
+  EXPECT_EQ(Suffix::Possible, Translate("optimizable"));
+  EXPECT_EQ(Suffix::PastTense, Translate("optimized"));
+  EXPECT_EQ(Suffix::Process, Translate("optimization"));
+  EXPECT_EQ(Suffix::InProgressAction, Translate("optimizing"));
+  EXPECT_EQ(Suffix::Unknown, Translate("optimizer"));
+  EXPECT_EQ(Suffix::Unknown, Translate("OPTIMIZABLE"));
+}
+
+TEST(StringSwitchTest, CaseInsensitiveEndsWith) {
+  enum class Suffix { Possible, PastTense, Process, InProgressAction, Unknown };
+
+  auto Translate = [](StringRef S) {
+    return llvm::StringSwitch<Suffix>(S)
+        .EndsWithLower("able", Suffix::Possible)
+        .EndsWithLower("ed", Suffix::PastTense)
+        .EndsWithLower("ation", Suffix::Process)
+        .EndsWithLower("ing", Suffix::InProgressAction)
+        .Default(Suffix::Unknown);
+  };
+
+  EXPECT_EQ(Suffix::Possible, Translate("optimizable"));
+  EXPECT_EQ(Suffix::Possible, Translate("OPTIMIZABLE"));
+  EXPECT_EQ(Suffix::PastTense, Translate("optimized"));
+  EXPECT_EQ(Suffix::Process, Translate("optimization"));
+  EXPECT_EQ(Suffix::InProgressAction, Translate("optimizing"));
+  EXPECT_EQ(Suffix::Unknown, Translate("optimizer"));
+}
+
+TEST(StringSwitchTest, CaseSensitiveCases) {
+  enum class OSType { Windows, Linux, Unknown };
+
+  auto Translate = [](StringRef S) {
+    return llvm::StringSwitch<OSType>(S)
+        .Cases("windows", "win32", "winnt", OSType::Windows)
+        .Cases("linux", "unix", "*nix", "posix", OSType::Linux)
+        .Default(OSType::Unknown);
+  };
+
+  EXPECT_EQ(OSType::Windows, Translate("windows"));
+  EXPECT_EQ(OSType::Windows, Translate("win32"));
+  EXPECT_EQ(OSType::Windows, Translate("winnt"));
+
+  EXPECT_EQ(OSType::Linux, Translate("linux"));
+  EXPECT_EQ(OSType::Linux, Translate("unix"));
+  EXPECT_EQ(OSType::Linux, Translate("*nix"));
+  EXPECT_EQ(OSType::Linux, Translate("posix"));
+
+  EXPECT_EQ(OSType::Unknown, Translate("Windows"));
+  EXPECT_EQ(OSType::Unknown, Translate(""));
+}
+
+TEST(StringSwitchTest, CaseInensitiveCases) {
+  enum class OSType { Windows, Linux, Unknown };
+
+  auto Translate = [](StringRef S) {
+    return llvm::StringSwitch<OSType>(S)
+        .CasesLower("windows", "win32", "winnt", OSType::Windows)
+        .CasesLower("linux", "unix", "*nix", "posix", OSType::Linux)
+        .Default(OSType::Unknown);
+  };
+
+  EXPECT_EQ(OSType::Windows, Translate("windows"));
+  EXPECT_EQ(OSType::Windows, Translate("win32"));
+  EXPECT_EQ(OSType::Windows, Translate("winnt"));
+
+  EXPECT_EQ(OSType::Linux, Translate("linux"));
+  EXPECT_EQ(OSType::Linux, Translate("unix"));
+  EXPECT_EQ(OSType::Linux, Translate("*nix"));
+  EXPECT_EQ(OSType::Linux, Translate("posix"));
+
+  EXPECT_EQ(OSType::Windows, Translate("Windows"));
+  EXPECT_EQ(OSType::Linux, Translate("Linux"));
+
+  EXPECT_EQ(OSType::Unknown, Translate(""));
+}
+
+TEST(StringSwitchTest, Expected) {
+  auto Translate = [](StringRef S) {
+    return llvm::StringSwitch<bool>(S)
+        .Cases("true", "on", "yes", "1", "totally!", true)
+        .Cases("false", "off", "no", "0", "wut?", false)
+        .Expect();
+  };
+
+  EXPECT_EXPECTED(true, Translate("true"));
+  EXPECT_EXPECTED(true, Translate("on"));
+  EXPECT_EXPECTED(true, Translate("yes"));
+  EXPECT_EXPECTED(true, Translate("1"));
+  EXPECT_EXPECTED(true, Translate("totally!"));
+
+  EXPECT_EXPECTED(false, Translate("false"));
+  EXPECT_EXPECTED(false, Translate("off"));
+  EXPECT_EXPECTED(false, Translate("no"));
+  EXPECT_EXPECTED(false, Translate("0"));
+  EXPECT_EXPECTED(false, Translate("wut?"));
+
+  EXPECT_UNEXPECTED(Translate(""));
+  EXPECT_UNEXPECTED(Translate("invalid"));
+}