[Lldb-commits] [PATCH] D65185: Let tablegen generate property definitions

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D65185#1599215 <https://reviews.llvm.org/D65185#1599215>, @JDevlieghere wrote:

> In D65185#1598586 <https://reviews.llvm.org/D65185#1598586>, @labath wrote:
>
> > My dream is to one day be able to define a property by simply declaring a variable somewhere (say: `Property<T> Foo(ParentProperty, "foo", "description of foo", DefaultValue);`), and that one could just get/set them via something like `context[Foo] = new_value`. In that world, to tablegenning would hopefully be necessary, but that world is still pretty far away, and your approach does help with eliminating the redundancy, so maybe it's worth doing anyway... I dunno...
> >
> > Regarding the patch itself, I have two questions/comments:
> >
> > - you put all property definitions into a single file, including those coming from "plugins". This is kind of bad as the knowledge of plugin internals leaks out. It may not be too bad, if the same effect could be achieved by just putting the definitions into a separate file and running tablegen twice, and this is just a way of avoiding that. It looks like that is the case here, but I'm not 100% sure. It's something to keep an eye on, at least, particularly, if we ever want to make "real" plugins.
>
>
> Yep, but I'm also happy to have separate files for the plugins. They have only a few options, so maybe tablegen is fast enough that it doesn't really matter.


Cool. Glad we're on the same page.

> 
> 
>> - I'm wondering if the same effect could not be achieved in a more low-cost way via a `.def` header file. I believe @aprantl had a patch like that at one point. The .def file could just contain something like: `LLDB_PROPERTY(eFoo, "foo", "description", default)`. When you want to define the enum, you just have the macro expand to "eFoo", when you define the property itself, you have it expand to the whole `{eFoo, ...}` blurb... The advantage of tablegen is that it allows you to define the property list via some non-trivial algorithm, but it's not clear to me whether this is needed/useful here...
> 
> The `def` file is really nice when everything has the exact same structure, like things in `Dwarf.def`. The advantage of using tablegen here is that you can omit fields, like the default string value when they're not needed and do some sanity checking. It also has the advantage we can change the representation down the road, without having to modify the tablegen files.

.def files can omit fields too: `#define BOOL_PROPERTY(name, global, default, desc) PROPERTY(name, OptionValue::eTypeBoolean, global, default, nullptr, {}, desc)`. Some sanity checking sounds like it could be useful, but I'm not exactly sure what kind of checks you have in mind. Being able to change the representation is nice, but I expect most of those changes would also be achievable with the def files. More radical changes (like the variable thing I mentioned) would probably require changes regardless of how the properties are generated...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65185