[Lldb-commits] [PATCH] D65386: [lldb][NFC] Use an enum instead of chars when handling options [WIP]

Jonas Devlieghere via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Mon Jul 29 13:32:43 PDT 2019


JDevlieghere added a comment.

In D65386#1604927 <https://reviews.llvm.org/D65386#1604927>, @jingham wrote:

> It worries me a little bit that we are making it harder and harder to figure out "where does the option for "-t" get stored once this CommandObject's options have been parsed.  Can you show the steps I would have to go through to get from "-f" to OptionEnumSettingsSet::Force or whatever.


TableGen's goal is "... to help a human develop and maintain records of domain-specific information." I think having a bit of code generation is nice, as longs as things don't become magical. The current patch is fine I think and I think Pavel's suggestion sounds good, as long as it's well documented. It's also the reason I suggest using the record names, instead of converting the name to CamelCase, so that you can at least grep for them.

In D65386#1605054 <https://reviews.llvm.org/D65386#1605054>, @clayborg wrote:

> Yeah, that has been my experience with table gen stuff. Does the table gen stuff generate code that exists in the build folder, like headers and/or C++ code? Navigating using an IDE often fails on these because the build system doesn't know about it directly. Any way to generate code in the source tree from the .inc files that we check in? Then if anyone modifies the .inc file it would regenerate (during the normal build process which could depend on the .inc _and_ on the .h or .cpp file that gets generated) the .h and .cpp file in the source tree and they would show up as modifications.


I really wouldn't want the generated code to be checked-in. The whole idea is that you don't really care what the (generated) table looks like, you only care about its content, which is defined in a human-readable way in the `.td` file. For me, TabelGen is about **records** rather than generating arbitrary code. Including some boiler plate is good (like the array definitions), but the surrounding code needs to be understandable, without having to look at the TableGen backend or generated code.



================
Comment at: lldb/utils/TableGen/LLDBOptionDefEmitter.cpp:211
+
+  std::string CamelCaseID = ToCamelCase(Command);
+
----------------
Can we use the option name instead, like I did for the properties? Or would that cause conflicts? 


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65386





More information about the lldb-commits mailing list