On Thu Apr 4 14:44:01 2024 +0000, Gabriel Ivăncescu wrote:
The goal was to simplify the design for prop handlers in the rest of the code. The simplest for me was just like this for common cases:
if(!get_prop(..., &prop)) /* not found */ prop.vtbl->method(...)
In this design I wanted to have the vtbl tied to the specific prop. This is obviously needed for this: even an object with indexed props needs at least two operations on different props, the indexed ones and the dispex (named) props, which have different methods. If I put it in `builtin_info_t`, then I'd either have to special case named props (they apply to all objects), or duplicate that in objects that have indexed props. If I make it optional, I'd also have to check for the props methods being NULL and fall back to named props (unless I don't make it optional so that every object needs to have one). I mean, think of all the objects not having indexed props. Probably with helpers, else it becomes tedious. But honestly, if this is the case, the whole thing seems a little pointless. Wasn't the point of this to *avoid* checks like that everywhere? Right now all props are filled with the vtbl in only few places, the rest of code doesn't care. Even for objects not having indexed props.
There is a whole range of solutions between checking for class ID in dispex functions and per-property vtbl. By following your logic, from dispex.c point of view, you may have at most two types of properties: internal or external. Representing that distinction doesn't need a per-property vtbl.
Looking at it from a different perspective, we already handle multiple types of properties represented by `prop_type_t`. One way of thinking about external properties is that it's another type on that list, except there is no `dispex_prop_t` to store it, so it's a meaning of NULL prop instead. (Maybe we'd even want to optionally offer storage for external implementations, for example for overrides, in which case we may even want `dispex_prop_t` entry for that.)