labath added a comment. In D65185#1599423 <https://reviews.llvm.org/D65185#1599423>, @JDevlieghere wrote:
> In D65185#1599262 <https://reviews.llvm.org/D65185#1599262>, @labath wrote: > > > .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... > > > I agree with you and I'm not opposed to def-files at all. I think tablegen > and `.def` files have different trade-offs and while the latter could > probably work for properties, I have the feeling that tablegen is a better > fit. The things are mentioned before are just a few things that came to mind. > > To give an example of sanity checking: this isn't in the patch (yet) but with > tablegen we can ensure that every option has either a default unsigned or > string value. In the table you can't differentiate between a default `0` and > an explicit default value of `0`. If you used something like the BOOL_PROPERTY macro from my previous comment, then there would be no way to explicitly specify a default string value for a boolean property (and similarly for other property types too). Overall, I'm not strongly opposed to this, but I am not convinced that the added complexity of tablegen ("#including a .def file" vs. "running a functional program which generates a def file, which is then #included") is worth the benefits. I suggest getting someone else to review this to break the stalemate. :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65185/new/ https://reviews.llvm.org/D65185 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
