Improve internal StringSwitch code gen with Optional<T>

[davezarzycki]

Summary

StringSwitch currently fakes Optional<T> by storing a pointer to the result. The problem with this approach is that it forces the compiler to store all case results on the stack before entering the string switch just so the address of the result can be temporarily used.

[davezarzycki]

Hi Doug! Looks like you created StringSwitch back in 2009. :-)

[doug.gregor]

Heh, I was probably thinking that one might want to produce references, but I don’t anyone does. I do think it would be valuable to make the parameters “T value” instead of “const T value”, and std::move them into place when we find a match.

[davezarzycki]

Thanks for the feedback Doug. I can certainly make the std::move change. Separately and to the best of my knowledge, I don't have commit privileges to LLVM proper, so somebody else will need to merge this. Thanks :-)

[davezarzycki]

Made requested changes by Doug

include/llvm/ADT/StringSwitch.h

 //===--- StringSwitch.h - Switch-on-literal-string Construct --------------===/
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //===----------------------------------------------------------------------===/
 //
 //  This file implements the StringSwitch template, which mimics a switch()
 //  statement whose cases are string literals.
 //
 //===----------------------------------------------------------------------===/
 #ifndef LLVM_ADT_STRINGSWITCH_H
 #define LLVM_ADT_STRINGSWITCH_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/Compiler.h"
 #include <cassert>
 #include <cstring>
 
 namespace llvm {
 
 /// \brief A switch()-like statement whose cases are string literals.
 ///

 ///   .Cases("violet", "purple", Violet)
 ///   .Default(UnknownColor);
 /// \endcode
 template<typename T, typename R = T>
 class StringSwitch {
   /// \brief The string we are matching.
   StringRef Str;
 
-  /// \brief The pointer to the result of this switch statement, once known,
-  /// null before that.
-  const T *Result;
+  /// \brief The result of this switch statement.
+  Optional<T> Result;
 
 public:
   LLVM_ATTRIBUTE_ALWAYS_INLINE
   explicit StringSwitch(StringRef S)
-  : Str(S), Result(nullptr) { }
+  : Str(S), Result() { }
 
   // StringSwitch is not copyable.
   StringSwitch(const StringSwitch &) = delete;
   void operator=(const StringSwitch &) = delete;
 
   StringSwitch(StringSwitch &&other) {
     *this = std::move(other);
   }
   StringSwitch &operator=(StringSwitch &&other) {
     Str = other.Str;
     Result = other.Result;
     return *this;
   }
 
   ~StringSwitch() = default;
 
   // Case-sensitive case matchers
   template<unsigned N>
   LLVM_ATTRIBUTE_ALWAYS_INLINE
-  StringSwitch& Case(const char (&S)[N], const T& Value) {
+  StringSwitch& Case(const char (&S)[N], T Value) {
     assert(N);
     if (!Result && N-1 == Str.size() &&
         (N == 1 || std::memcmp(S, Str.data(), N-1) == 0)) {
-      Result = &Value;
+      Result = std::move(Value);
     }
     return *this;
   }
 
   template<unsigned N>
   LLVM_ATTRIBUTE_ALWAYS_INLINE
-  StringSwitch& EndsWith(const char (&S)[N], const T &Value) {
+  StringSwitch& EndsWith(const char (&S)[N], T Value) {
     assert(N);
     if (!Result && Str.size() >= N-1 &&
         (N == 1 || std::memcmp(S, Str.data() + Str.size() + 1 - N, N-1) == 0)) {
-      Result = &Value;
+      Result = std::move(Value);
     }
     return *this;
   }
 
   template<unsigned N>
   LLVM_ATTRIBUTE_ALWAYS_INLINE
-  StringSwitch& StartsWith(const char (&S)[N], const T &Value) {
+  StringSwitch& StartsWith(const char (&S)[N], T Value) {
     assert(N);
     if (!Result && Str.size() >= N-1 &&
         (N == 1 || std::memcmp(S, Str.data(), N-1) == 0)) {
-      Result = &Value;
+      Result = std::move(Value);
     }
     return *this;
   }
 
   template<unsigned N0, unsigned N1>
   LLVM_ATTRIBUTE_ALWAYS_INLINE
-  StringSwitch &Cases(const char (&S0)[N0], const char (&S1)[N1],
-                      const T& Value) {
+  StringSwitch &Cases(const char (&S0)[N0], const char (&S1)[N1], T Value) {
     return Case(S0, Value).Case(S1, Value);
   }
 
   template<unsigned N0, unsigned N1, unsigned N2>
   LLVM_ATTRIBUTE_ALWAYS_INLINE
   StringSwitch &Cases(const char (&S0)[N0], const char (&S1)[N1],
-                      const char (&S2)[N2], const T& Value) {
+                      const char (&S2)[N2], T Value) {
     return Case(S0, Value).Cases(S1, S2, Value);
   }
 
   template<unsigned N0, unsigned N1, unsigned N2, unsigned N3>
   LLVM_ATTRIBUTE_ALWAYS_INLINE
   StringSwitch &Cases(const char (&S0)[N0], const char (&S1)[N1],
-                      const char (&S2)[N2], const char (&S3)[N3],
-                      const T& Value) {
+                      const char (&S2)[N2], const char (&S3)[N3], T Value) {
     return Case(S0, Value).Cases(S1, S2, S3, Value);
   }
 
   template<unsigned N0, unsigned N1, unsigned N2, unsigned N3, unsigned N4>
   LLVM_ATTRIBUTE_ALWAYS_INLINE
   StringSwitch &Cases(const char (&S0)[N0], const char (&S1)[N1],
                       const char (&S2)[N2], const char (&S3)[N3],
-                      const char (&S4)[N4], const T& Value) {
+                      const char (&S4)[N4], T Value) {
     return Case(S0, Value).Cases(S1, S2, S3, S4, Value);
   }
 
   template <unsigned N0, unsigned N1, unsigned N2, unsigned N3, unsigned N4,
             unsigned N5>
   LLVM_ATTRIBUTE_ALWAYS_INLINE
   StringSwitch &Cases(const char (&S0)[N0], const char (&S1)[N1],
                       const char (&S2)[N2], const char (&S3)[N3],
-                      const char (&S4)[N4], const char (&S5)[N5],
-                      const T &Value) {
+                      const char (&S4)[N4], const char (&S5)[N5], T Value) {
     return Case(S0, Value).Cases(S1, S2, S3, S4, S5, Value);
   }
 
   template <unsigned N0, unsigned N1, unsigned N2, unsigned N3, unsigned N4,
             unsigned N5, unsigned N6>
   LLVM_ATTRIBUTE_ALWAYS_INLINE
   StringSwitch &Cases(const char (&S0)[N0], const char (&S1)[N1],
                       const char (&S2)[N2], const char (&S3)[N3],
                       const char (&S4)[N4], const char (&S5)[N5],
-                      const char (&S6)[N6], const T &Value) {
+                      const char (&S6)[N6], T Value) {
     return Case(S0, Value).Cases(S1, S2, S3, S4, S5, S6, Value);
   }
 
   template <unsigned N0, unsigned N1, unsigned N2, unsigned N3, unsigned N4,
             unsigned N5, unsigned N6, unsigned N7>
   LLVM_ATTRIBUTE_ALWAYS_INLINE
   StringSwitch &Cases(const char (&S0)[N0], const char (&S1)[N1],
                       const char (&S2)[N2], const char (&S3)[N3],
                       const char (&S4)[N4], const char (&S5)[N5],
-                      const char (&S6)[N6], const char (&S7)[N7],
-                      const T &Value) {
+                      const char (&S6)[N6], const char (&S7)[N7], T Value) {
     return Case(S0, Value).Cases(S1, S2, S3, S4, S5, S6, S7, Value);
   }
 
   template <unsigned N0, unsigned N1, unsigned N2, unsigned N3, unsigned N4,
             unsigned N5, unsigned N6, unsigned N7, unsigned N8>
   LLVM_ATTRIBUTE_ALWAYS_INLINE
   StringSwitch &Cases(const char (&S0)[N0], const char (&S1)[N1],
                       const char (&S2)[N2], const char (&S3)[N3],
                       const char (&S4)[N4], const char (&S5)[N5],
                       const char (&S6)[N6], const char (&S7)[N7],
-                      const char (&S8)[N8], const T &Value) {
+                      const char (&S8)[N8], T Value) {
     return Case(S0, Value).Cases(S1, S2, S3, S4, S5, S6, S7, S8, Value);
   }
 
   template <unsigned N0, unsigned N1, unsigned N2, unsigned N3, unsigned N4,
             unsigned N5, unsigned N6, unsigned N7, unsigned N8, unsigned N9>
   LLVM_ATTRIBUTE_ALWAYS_INLINE
   StringSwitch &Cases(const char (&S0)[N0], const char (&S1)[N1],
                       const char (&S2)[N2], const char (&S3)[N3],
                       const char (&S4)[N4], const char (&S5)[N5],
                       const char (&S6)[N6], const char (&S7)[N7],
-                      const char (&S8)[N8], const char (&S9)[N9],
-                      const T &Value) {
+                      const char (&S8)[N8], const char (&S9)[N9], T Value) {
     return Case(S0, Value).Cases(S1, S2, S3, S4, S5, S6, S7, S8, S9, Value);
   }
 
   // Case-insensitive case matchers.
   template <unsigned N>
   LLVM_ATTRIBUTE_ALWAYS_INLINE StringSwitch &CaseLower(const char (&S)[N],
-                                                       const T &Value) {
+                                                       T Value) {
     if (!Result && Str.equals_lower(StringRef(S, N - 1)))
-      Result = &Value;
+      Result = std::move(Value);
 
     return *this;
   }
 
   template <unsigned N>
   LLVM_ATTRIBUTE_ALWAYS_INLINE StringSwitch &EndsWithLower(const char (&S)[N],
-                                                           const T &Value) {
+                                                           T Value) {
     if (!Result && Str.endswith_lower(StringRef(S, N - 1)))
-      Result = &Value;
+      Result = std::move(Value);
 
     return *this;
   }
 
   template <unsigned N>
   LLVM_ATTRIBUTE_ALWAYS_INLINE StringSwitch &StartsWithLower(const char (&S)[N],
-                                                             const T &Value) {
+                                                             T Value) {
     if (!Result && Str.startswith_lower(StringRef(S, N - 1)))
-      Result = &Value;
+      Result = std::move(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) {
+  CasesLower(const char (&S0)[N0], const char (&S1)[N1], 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) {
+             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) {
+             const char (&S3)[N3], 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) {
+             const char (&S3)[N3], const char (&S4)[N4], T Value) {
     return CaseLower(S0, Value).CasesLower(S1, S2, S3, S4, Value);
   }
 
   LLVM_ATTRIBUTE_ALWAYS_INLINE
-  R Default(const T &Value) const {
+  R Default(T Value) const {
     if (Result)
       return *Result;
     return Value;
   }
 
   LLVM_ATTRIBUTE_ALWAYS_INLINE
   operator R() const {
     assert(Result && "Fell off the end of a string-switch");
     return *Result;
   }
 };
 
 } // end namespace llvm
 
 #endif // LLVM_ADT_STRINGSWITCH_H