[Lldb-commits] [PATCH] D65489: Tablegen option enum value elements

Wed Jul 31 00:17:04 PDT 2019

labath added a comment.

In D65489#1607807 <https://reviews.llvm.org/D65489#1607807>, @JDevlieghere wrote:

> In D65489#1607801 <https://reviews.llvm.org/D65489#1607801>, @labath wrote:
>
> > Why do we need a separate backend for this? Couldn't this be emitted as a part of the same `#include "XXXProperties.inc"` which defines the property definition? The two logically belong together, and the only reason they were separate variables in the first place was the limitations of constexpr global variables...
>
>
> I'm not sure what you mean by that last sentence, maybe because I don't know the history? Can you elaborate a bit on the "separate variables" part?
>
> To answer your question: the `OptionEnumValueElement`s are used both by properties and command options. Technically it is possible to put them in the same `.td` files, and emit them in the same `.inc` file. The actual code wouldn't change, just the way we invoke: instead of being its own backend, it would be called from both existing backends. I don't have a strong preference for either approach.

Ah, I'm sorry. Somehow I didn't get what the " used by both properties and options" part means. I do now, and I agree that in this case, it does not make sense to generate these as a part of the properties definition.

However, I'm not sure it then makes sense to generate them at all. I mean, it's not like there's any redundancy (like it was for option definitions) in the `OptionEnumValueElement` struct, and it's pretty obvious what the individual fields mean even without naming them. And the tablegenning definitely increases the barrier for understanding the code. Is there any follow-up work or cleanups that would not be possible if this just stays a plain C array?

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

https://reviews.llvm.org/D65489