On Wed, Sep 20, 2023 at 09:23:46PM +0200, Thomas Haller wrote:
> On Wed, 2023-09-20 at 20:10 +0200, Pablo Neira Ayuso wrote:
> > On Wed, Sep 20, 2023 at 04:26:04PM +0200, Thomas Haller wrote:
> > > Flags are not always bad. For example, as a function argument they
> > > allow
> > > easier extension in the future. But with datatype's "flags"
> > > argument and
> > > enum datatype_flags there are no advantages of this approach.
> > >
> > > - replace DTYPE_F_PREFIX with a "bool f_prefix" field. This could
> > > even
> > > be a bool:1 bitfield if we cared to represent the information
> > > with
> > > one bit only. For now it's not done because that would not help
> > > reducing
> > > the size of the struct, so a bitfield is less preferable.
> > >
> > > - instead of DTYPE_F_ALLOC, use the refcnt of zero to represent
> > > static
> > > instances. Drop this redundant flag.
> >
> > Not sure I want to rely on refcnt to zero to identify dynamic
> > datatypes. I think we need to consolidate datatype_set() to be used
> > not only where this deals with dynamic datatypes, it might help
> > improve traceability of datatype assignment.
>
> I don't understand. Could you elaborate about datatype_set()?
I wonder if we could use datatype_set() to attach static datatypes
too, instead of manually attaching datatypes, such as:
expr->dtype = &integer_type;
in case of future extensions, using consistently this helper function
will help to identify datatype attachments.
> Btw, for dynamically allocated instances the refcnt is always positive,
> and for static ones it's always zero. The DTYPE_F_ALLOC flag is
> redundant.
That is a correct observation, but a (hipothetical) subtle bug in
refcnt might lead to a dynamic datatype get to refcnt to zero, and
that might be harder to track?
Let me have a look if I can come up with some counter proposal to get
rid of this flag, I would prefer not to infer the datatype class from
reference counter value.
