[ADT] Allow arbitrary number of cases in StringSwitch #163117
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-adt Author: Jakub Kuderski (kuhar) ChangesPrior to this patch,
Instead, Generalize I plan to eventually deprecate the manually-enumerate overloads of Full diff: https://github.com/llvm/llvm-project/pull/163117.diff 2 Files Affected:
diff --git a/llvm/include/llvm/ADT/StringSwitch.h b/llvm/include/llvm/ADT/StringSwitch.h
index 0ce7c57a358f2..a96535cd077fd 100644
--- a/llvm/include/llvm/ADT/StringSwitch.h
+++ b/llvm/include/llvm/ADT/StringSwitch.h
@@ -17,6 +17,7 @@
#include "llvm/Support/ErrorHandling.h"
#include <cassert>
#include <cstring>
+#include <initializer_list>
#include <optional>
namespace llvm {
@@ -85,55 +86,60 @@ class StringSwitch {
return *this;
}
+ StringSwitch &Cases(std::initializer_list<StringLiteral> CaseStrings,
+ T Value) {
+ return CasesImpl(Value, CaseStrings);
+ }
+
StringSwitch &Cases(StringLiteral S0, StringLiteral S1, T Value) {
- return CasesImpl(Value, S0, S1);
+ return CasesImpl(Value, {S0, S1});
}
StringSwitch &Cases(StringLiteral S0, StringLiteral S1, StringLiteral S2,
T Value) {
- return CasesImpl(Value, S0, S1, S2);
+ return CasesImpl(Value, {S0, S1, S2});
}
StringSwitch &Cases(StringLiteral S0, StringLiteral S1, StringLiteral S2,
StringLiteral S3, T Value) {
- return CasesImpl(Value, S0, S1, S2, S3);
+ return CasesImpl(Value, {S0, S1, S2, S3});
}
StringSwitch &Cases(StringLiteral S0, StringLiteral S1, StringLiteral S2,
StringLiteral S3, StringLiteral S4, T Value) {
- return CasesImpl(Value, S0, S1, S2, S3, S4);
+ return CasesImpl(Value, {S0, S1, S2, S3, S4});
}
StringSwitch &Cases(StringLiteral S0, StringLiteral S1, StringLiteral S2,
StringLiteral S3, StringLiteral S4, StringLiteral S5,
T Value) {
- return CasesImpl(Value, S0, S1, S2, S3, S4, S5);
+ return CasesImpl(Value, {S0, S1, S2, S3, S4, S5});
}
StringSwitch &Cases(StringLiteral S0, StringLiteral S1, StringLiteral S2,
StringLiteral S3, StringLiteral S4, StringLiteral S5,
StringLiteral S6, T Value) {
- return CasesImpl(Value, S0, S1, S2, S3, S4, S5, S6);
+ return CasesImpl(Value, {S0, S1, S2, S3, S4, S5, S6});
}
StringSwitch &Cases(StringLiteral S0, StringLiteral S1, StringLiteral S2,
StringLiteral S3, StringLiteral S4, StringLiteral S5,
StringLiteral S6, StringLiteral S7, T Value) {
- return CasesImpl(Value, S0, S1, S2, S3, S4, S5, S6, S7);
+ return CasesImpl(Value, {S0, S1, S2, S3, S4, S5, S6, S7});
}
StringSwitch &Cases(StringLiteral S0, StringLiteral S1, StringLiteral S2,
StringLiteral S3, StringLiteral S4, StringLiteral S5,
StringLiteral S6, StringLiteral S7, StringLiteral S8,
T Value) {
- return CasesImpl(Value, S0, S1, S2, S3, S4, S5, S6, S7, S8);
+ return CasesImpl(Value, {S0, S1, S2, S3, S4, S5, S6, S7, S8});
}
StringSwitch &Cases(StringLiteral S0, StringLiteral S1, StringLiteral S2,
StringLiteral S3, StringLiteral S4, StringLiteral S5,
StringLiteral S6, StringLiteral S7, StringLiteral S8,
StringLiteral S9, T Value) {
- return CasesImpl(Value, S0, S1, S2, S3, S4, S5, S6, S7, S8, S9);
+ return CasesImpl(Value, {S0, S1, S2, S3, S4, S5, S6, S7, S8, S9});
}
// Case-insensitive case matchers.
@@ -156,23 +162,28 @@ class StringSwitch {
return *this;
}
+ StringSwitch &CasesLower(std::initializer_list<StringLiteral> CaseStrings,
+ T Value) {
+ return CasesLowerImpl(Value, CaseStrings);
+ }
+
StringSwitch &CasesLower(StringLiteral S0, StringLiteral S1, T Value) {
- return CasesLowerImpl(Value, S0, S1);
+ return CasesLowerImpl(Value, {S0, S1});
}
StringSwitch &CasesLower(StringLiteral S0, StringLiteral S1, StringLiteral S2,
T Value) {
- return CasesLowerImpl(Value, S0, S1, S2);
+ return CasesLowerImpl(Value, {S0, S1, S2});
}
StringSwitch &CasesLower(StringLiteral S0, StringLiteral S1, StringLiteral S2,
StringLiteral S3, T Value) {
- return CasesLowerImpl(Value, S0, S1, S2, S3);
+ return CasesLowerImpl(Value, {S0, S1, S2, S3});
}
StringSwitch &CasesLower(StringLiteral S0, StringLiteral S1, StringLiteral S2,
StringLiteral S3, StringLiteral S4, T Value) {
- return CasesLowerImpl(Value, S0, S1, S2, S3, S4);
+ return CasesLowerImpl(Value, {S0, S1, S2, S3, S4});
}
[[nodiscard]] R Default(T Value) {
@@ -211,16 +222,21 @@ class StringSwitch {
return false;
}
- template <typename... Args> StringSwitch &CasesImpl(T &Value, Args... Cases) {
+ StringSwitch &CasesImpl(T &Value,
+ std::initializer_list<StringLiteral> Cases) {
// Stop matching after the string is found.
- (... || CaseImpl(Value, Cases));
+ for (StringLiteral S : Cases)
+ if (CaseImpl(Value, S))
+ break;
return *this;
}
- template <typename... Args>
- StringSwitch &CasesLowerImpl(T &Value, Args... Cases) {
+ StringSwitch &CasesLowerImpl(T &Value,
+ std::initializer_list<StringLiteral> Cases) {
// Stop matching after the string is found.
- (... || CaseLowerImpl(Value, Cases));
+ for (StringLiteral S : Cases)
+ if (CaseLowerImpl(Value, S))
+ break;
return *this;
}
};
diff --git a/llvm/unittests/ADT/StringSwitchTest.cpp b/llvm/unittests/ADT/StringSwitchTest.cpp
index bcb1521baa287..0fbf37153593e 100644
--- a/llvm/unittests/ADT/StringSwitchTest.cpp
+++ b/llvm/unittests/ADT/StringSwitchTest.cpp
@@ -153,13 +153,14 @@ TEST(StringSwitchTest, EndsWithLower) {
}
TEST(StringSwitchTest, Cases) {
- enum class OSType { Windows, Linux, Unknown };
+ enum class OSType { Windows, Linux, MacOS, Unknown };
auto Translate = [](StringRef S) {
return llvm::StringSwitch<OSType>(S)
.Cases(StringLiteral::withInnerNUL("wind\0ws"), "win32", "winnt",
OSType::Windows)
.Cases("linux", "unix", "*nix", "posix", OSType::Linux)
+ .Cases({"macos", "osx"}, OSType::MacOS)
.Default(OSType::Unknown);
};
@@ -172,21 +173,26 @@ TEST(StringSwitchTest, Cases) {
EXPECT_EQ(OSType::Linux, Translate("*nix"));
EXPECT_EQ(OSType::Linux, Translate("posix"));
+ EXPECT_EQ(OSType::MacOS, Translate("macos"));
+ EXPECT_EQ(OSType::MacOS, Translate("osx"));
+
// Note that the whole null-terminator embedded string is required for the
// case to match.
EXPECT_EQ(OSType::Unknown, Translate("wind"));
EXPECT_EQ(OSType::Unknown, Translate("Windows"));
+ EXPECT_EQ(OSType::Unknown, Translate("MacOS"));
EXPECT_EQ(OSType::Unknown, Translate(""));
}
TEST(StringSwitchTest, CasesLower) {
- enum class OSType { Windows, Linux, Unknown };
+ enum class OSType { Windows, Linux, MacOS, Unknown };
auto Translate = [](StringRef S) {
return llvm::StringSwitch<OSType>(S)
.CasesLower(StringLiteral::withInnerNUL("wind\0ws"), "win32", "winnt",
OSType::Windows)
.CasesLower("linux", "unix", "*nix", "posix", OSType::Linux)
+ .CasesLower({"macos", "osx"}, OSType::MacOS)
.Default(OSType::Unknown);
};
@@ -202,6 +208,9 @@ TEST(StringSwitchTest, CasesLower) {
EXPECT_EQ(OSType::Windows, Translate(llvm::StringRef("wind\0ws", 7)));
EXPECT_EQ(OSType::Linux, Translate("linux"));
+ EXPECT_EQ(OSType::MacOS, Translate("macOS"));
+ EXPECT_EQ(OSType::MacOS, Translate("OSX"));
+
EXPECT_EQ(OSType::Unknown, Translate("wind"));
EXPECT_EQ(OSType::Unknown, Translate(""));
}
|
arsenm
approved these changes
Oct 13, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/17431 Here is the relevant piece of the build log for the reference |
akadutta
pushed a commit
to akadutta/llvm-project
that referenced
this pull request
Oct 14, 2025
Prior to this patch, `.Cases` and `.CasesLower` were **not** variadic functions and accepted a fixed number of arguments: up to 10 for the former and up to 5 for the latter. This is close to unfixable with variadic templates because of the following constraints: * There would be a variadic number of cases and the single value would be at the very end. For a parameter pack approach to work, we would want the value to be the head and the cases strings to follow, but rotating a parameter pack is quite awkward and requires heavy template metaprogramming. * Even if we did rotate the parameter pack, the existing function signature forces conversion of cases to `StringLiteral` and the value to `T`. This is hard to achieve with variadic templates because we can't specify that all cases must be of type `StringLiteral`, and doing the conversion later won't work because of the attribute enable_if in `StringLiteral`'s constructor that doesn't work on non-literal character arrays. * Refactoring all the existing code to put the value first is very intrusive and bugprone when the value parameter is also a string literal. Instead, generalize `.Cases` and `.CasesLower` and make the code manageable by taking an initializer list. The added benefit is that this clearly delineates where case strings end and where the value begins, which can be hard to tell when the value parameter is also a string literal. I plan to eventually deprecate the manually-enumerated overloads of `.Cases.` and `.CasesLower`.
kuhar
added a commit
that referenced
this pull request
Oct 15, 2025
…163405) Switch to the `.Cases({S0, S1, ...}, Value)` overload instead. Update existing uses affected by the deprecation and the surrounding code (for consistency). This is a part of a larger cleanup of StringSwitch. The goal is to eventually deprecate all manually-enumerated overloads of Cases with a hardcoded number of case values (in favor of passing them via an initializer list). You can find the full explanation here: #163117. Start small (6+ arguments) to keep the number of changes manageable.
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Oct 15, 2025
…ted. NFC. (#163405)
Switch to the `.Cases({S0, S1, ...}, Value)` overload instead.
Update existing uses affected by the deprecation and the surrounding
code (for consistency).
This is a part of a larger cleanup of StringSwitch. The goal is to
eventually deprecate all manually-enumerated overloads of Cases with a
hardcoded number of case values (in favor of passing them via an
initializer list). You can find the full explanation here:
llvm/llvm-project#163117.
Start small (6+ arguments) to keep the number of changes manageable.
kuhar
added a commit
to kuhar/llvm-project
that referenced
this pull request
Oct 20, 2025
Suggest the `initializer_list` overload instead. 4+ args is an arbitrary number that allows for incremental deprecation without having too update too many call sites. For more context, see llvm#163117.
kuhar
added a commit
that referenced
this pull request
Oct 20, 2025
Suggest the `initializer_list` overload instead. 4+ args is an arbitrary number that allows for incremental deprecation without having too update too many call sites. For more context, see #163117.
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Oct 20, 2025
…4276) Suggest the `initializer_list` overload instead. 4+ args is an arbitrary number that allows for incremental deprecation without having too update too many call sites. For more context, see llvm/llvm-project#163117.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prior to this patch,
.Casesand.CasesLowerwere not variadic functions and accepted a fixed number of arguments: up to 10 for the former and up to 5 for the latter.This is close to unfixable with variadic templates because of the following constraints:
StringLiteraland the value toT. This is hard to achieve with variadic templates because we can't specify that all cases must be of typeStringLiteral, and doing the conversion later won't work because of the attribute enable_if inStringLiteral's constructor that doesn't work on non-literal character arrays.Instead, generalize
.Casesand.CasesLowerand make the code manageable by taking an initializer list. The added benefit is that this clearly delineates where case strings end and where the value begins, which can be hard to tell when the value parameter is also a string literal.I plan to eventually deprecate the manually-enumerated overloads of
.Cases.and.CasesLower.