teemperor created this revision.
teemperor added a reviewer: JDevlieghere.
Herald added subscribers: lldb-commits, abidh.
Herald added a project: LLDB.
teemperor added a comment.
For the sake of completeness, that's how Clang would warn about unimplemented
options:
llvm-project/lldb/source/Commands/CommandObjectSettings.cpp:102:15: warning:
enumeration value 'Global' not handled in switch [-Wswitch]
switch (*opt) {
^
llvm-project/lldb/source/Commands/CommandObjectSettings.cpp:102:15: note: add
missing switch cases
switch (*opt) {
^
Currently in LLDB we handle options like this:
switch(short_option) {
case 'g': m_force = true; break;
case 'f': m_global = true; supportGlobal(); break;
default:
error.SetErrorStringWithFormat("unrecognized options '%c'",
short_option);
break;
This format has a two problems:
1. If we don't handle an option in this switch statement (because of a typo, a
changed short-option
name, or because someone just forgot to implement it), we only find out when we
have a test or user
that notices we get an error when using this option. There is no compile-time
verification of this.
2. It's not very easy to read unless you know all the short options for all
commands.
This patch makes our tablegen emit an enum that represents the options, which
changes the code above to this (using SettingsSet as an example):
auto opt = toSettingsSetEnum(short_option, error);
if (!opt)
return error;
switch (opt) {
case SettingsSet::Global: m_force = true; break;
case SettingsSet::Force: m_global = true; supportGlobal(); break;
// No default with error handling, already handled in toSettingsSetEnum.
// If you don't implement an option, this will trigger a compiler warning for
unhandled enum value.
}
NOTE: This is just a dummy patch to get some feedback if people are for some
reason opposed to this change
(which would save me from converting all our switch-cases to the new format). I
only changed the code for
`settings set` to demonstrate the change.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D65386
Files:
lldb/source/Commands/CommandObjectSettings.cpp
lldb/utils/TableGen/LLDBOptionDefEmitter.cpp
Index: lldb/utils/TableGen/LLDBOptionDefEmitter.cpp
===================================================================
--- lldb/utils/TableGen/LLDBOptionDefEmitter.cpp
+++ lldb/utils/TableGen/LLDBOptionDefEmitter.cpp
@@ -160,6 +160,44 @@
OS << "},\n";
}
+static std::string Capitalize(llvm::StringRef s) {
+ return s.substr(0, 1).upper() + s.substr(1).str();
+}
+static std::string ToCamelCase(llvm::StringRef In) {
+ llvm::SmallVector<StringRef, 4> Parts;
+ llvm::SplitString(In, Parts, "-_ ");
+ std::string Result;
+ for (llvm::StringRef S : Parts)
+ Result.append(Capitalize(S));
+ return Result;
+}
+
+static void emitEnumDeclaration(llvm::StringRef EnumName,
+ const std::vector<CommandOption> &Options,
+ raw_ostream &OS) {
+ OS << "namespace { enum class " << EnumName << " {\n";
+ for (const CommandOption &CO : Options)
+ OS << " " << ToCamelCase(CO.FullName) << ",\n";
+ OS << "}; }\n";
+}
+
+static void emitEnumSwitch(llvm::StringRef EnumName,
+ const std::vector<CommandOption> &Options,
+ raw_ostream &OS) {
+ OS << "static llvm::Optional<" << EnumName << "> to" << EnumName
+ << "(char c, Status error) {\n";
+ OS << " switch(c) {";
+ for (const CommandOption &CO : Options)
+ OS << " case '" << CO.ShortName << "': return " << EnumName
+ << "::" << ToCamelCase(CO.FullName) << ";\n";
+ OS << R"cpp(
+ default:
+ error.SetErrorStringWithFormat("unrecognized option '%c'", c);
+ return {};
+)cpp";
+ OS << " }\n}\n";
+}
+
/// Emits all option initializers to the raw_ostream.
static void emitOptions(std::string Command, std::vector<Record *> Records,
raw_ostream &OS) {
@@ -169,6 +207,9 @@
std::string ID = Command;
std::replace(ID.begin(), ID.end(), ' ', '_');
+
+ std::string CamelCaseID = ToCamelCase(Command);
+
// Generate the macro that the user needs to define before including the
// *.inc file.
std::string NeededMacro = "LLDB_OPTIONS_" + ID;
@@ -180,8 +221,13 @@
OS << "constexpr static OptionDefinition g_" + ID + "_options[] = {\n";
for (CommandOption &CO : Options)
emitOption(CO, OS);
- // We undefine the macro for the user like Clang's include files are doing it.
OS << "};\n";
+
+ std::string Enum = "OptionEnum" + CamelCaseID;
+ emitEnumDeclaration(Enum, Options, OS);
+ emitEnumSwitch(Enum, Options, OS);
+
+ // We undefine the macro for the user like Clang's include files are doing it.
OS << "#undef " << NeededMacro << "\n";
OS << "#endif // " << Command << " command\n\n";
}
Index: lldb/source/Commands/CommandObjectSettings.cpp
===================================================================
--- lldb/source/Commands/CommandObjectSettings.cpp
+++ lldb/source/Commands/CommandObjectSettings.cpp
@@ -94,19 +94,18 @@
Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
ExecutionContext *execution_context) override {
Status error;
- const int short_option = m_getopt_table[option_idx].val;
- switch (short_option) {
- case 'f':
+ auto opt = toOptionEnumSettingsSet(m_getopt_table[option_idx].val, error);
+ if (!opt)
+ return error;
+
+ switch (*opt) {
+ case OptionEnumSettingsSet::Force:
m_force = true;
break;
- case 'g':
+ case OptionEnumSettingsSet::Global:
m_global = true;
break;
- default:
- error.SetErrorStringWithFormat("unrecognized options '%c'",
- short_option);
- break;
}
return error;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
