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

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D65489#1607833 <https://reviews.llvm.org/D65489#1607833>, @labath wrote:

> 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?


I don't have any cleanups planned for now. My motivation is purely aesthetical: I don't like the `// clang-format off` markers and think the C arrays look messy with the multiline oddly broken up strings. Anyway, I just mention it for context. I don't want to push this through if the consensus is that this is overkill.


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

https://reviews.llvm.org/D65489