[Lldb-commits] [PATCH] D129703: [lldb] Refactor command option enum values (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.


================
Comment at: lldb/source/Commands/CommandObjectBreakpointCommand.cpp:25-54
-// FIXME: "script-type" needs to have its contents determined dynamically, so
-// somebody can add a new scripting language to lldb and have it pickable here
-// without having to change this enumeration by hand and rebuild lldb proper.
-static constexpr OptionEnumValueElement g_script_option_enumeration[] = {
-    {
-        eScriptLanguageNone,
-        "command",
----------------
clayborg wrote:
> instead of moving all of these global enum declarations into CommandOptionArgumentTable.h, can we register the enum type in a static Initalize method? Maybe leave this code here and then add a function that would register a given enum type with the command interpreter so they can be displayed?
> ```
> static void CommandObjectBreakpointCommandAdd::Initialize() {
>   CommandInterpreter::RegisterEnumerationOption(g_script_option_enumeration, "<script-language>");
> }
> ```
> Then we can leave these definitions where they live, but register then so they can be seen in the help?
That's a great question. 

At a technical level that wouldn't work because of the way the option definitions are currently generated. All these tables and their entries are built at compile time (i.e. they're all `constexpr`). I didn't dig into why that's necessary, but I'm assuming there is a reason for it. But even if we were to change that, these enums shouldn't belong to a specific command. Multiple commands can share these argument types so in that sense it only makes sense that their values are shared as well. Before this patch, it's totally possible to specify that something takes an argument <foo> and one commands accepts a certain set of enum values while the other accepts a totally different set. By centralizing them, there's a unique mapping between argument type and its values, which is exactly what we need to display them in the help. 


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

https://reviews.llvm.org/D129703