constant for empty prefix set

[Alexander Zubkov]

Adding new pathches (v2 - full patch, v1-v2 - incremental from previous):

1) Modified as_path_filter function - added additional parameter to
choose between using set or key parameter. Now empty set does not work
like a zero integer constant. Also thought about passing one struct
f_val parameter instead of those 3 parameters, but that requires
adding include of filter/data.h to nest/attrs.h, and that looks like
causing a chicken and an egg problem.

2) Added the case of empty T_SET to be promoted to T_PREFIX_SET in
f_const_promotion function. There is need to create an empty trie, I
used "f_new_trie(cfg_mem, 0)", but not sure cfg_mem is good to use
here. It works with simple tests. By the way, not completely sure that
during promotion, the value that is overwrited, is a copy of the
constant value (because it can be a global named constant) and not the
original value itself. I made some tests and looks like that after
assignment, the global constan value remains to be a T_SET, I hope
this is not because of some optimization.

3) Made filter/delete to act the same on T_PATH. For some reason
delete(path, integer) was allowed, but filter(path, integer) wasn't.

There are actually still problems with types:

4) I found out that passing arguments to the function does not check
actual types during runtime. So we can actually pass any type to a
function and it will even work, given that operators applied to the
variables are supported.

5) When prefix set variable is assigned an empty set, it is promoted
to the T_PREFIX_SET, of course, but after that it becomes not equal to
"[]", because the type is different. But that can be considered a
feature, not a bug.

6) Because of (4), as function arguments are not checked and are not
promoted, if we have an argument of type prefix set, and pass [] to
that function, it will remain of type T_SET. And if we try to assign
it to another prefix set variable, there will be a runtime error like
this:

bird: filters, line 12: Argument 1 of VAR_SET must be of type set, got type set

This "feature" affects quads also.

On Tue, Feb 22, 2022 at 5:18 AM Ondrej Zajicek <santiago at crfreenet.org> wrote:
>
> On Tue, Feb 22, 2022 at 03:02:55AM +0100, Alexander Zubkov wrote:
> > > Also to your previous question:
> > >
> > > > > I also think now - why at all do we need a typed empty set? I think it
> > > > > is possible to add an untyped empty set, that will act as a "joker"
> > > > > with any types. It should fit better to the current syntax now without
> > > > > introducing a lot of new constructions.
> > >
> > > It breaks static type controls or at least make them more complicated.
> > > Note that our types for sets are already pretty broken - we have just
> > > T_SET and T_PREFIX_SET in implementation, although different types of
> > > sets are used in filter language (e.g. 'int set' in variable definition)
> > > and matching types are tested just in runtime (based on set->from.type).
> > > But let's assume we would fix this, we would have types T_INT_SET,
> > > I_IP_SET, ..., T_PREFIX_SET for each set type.
> > >
> > > Then defining untyped empty set means it would be a new type (T_EMPTY_SET)
> > > different from others, it would require some exceptions in type checks
> > > (e.g. FI_VAR_SET just checks that type of value is the same as type of
> > > variable) and many expressions that accepts or returns value of just one
> > > type now would accepts or returns values of two different types. We would
> > > have to extend type checking system for this or implement some concept of
> > > subtyping or type coercion.
> > >
> >
> > Sure, I had the same concerns about this idea with T_EMPTY_SET. But on the
> > other hand, with an empty T_SET we still need to make an exception at least
> > for prefixes. And if, as you said, there are more types of sets in the
> > future, than there will be still more exceptions. So in some circumstances,
> > I think, both ways will have around the same number of exceptions. But
> > currently, the empty T_SET looks easier of course.
> >
> > So, are you against an untyped empty set constant given the mentioned
> > problems? Or you are unhappy about the complexity of implementing and
> > maintaining that idea, but anyway consider it acceptable?
>
> I checked how IP/QUAD situation is solved and it is cleaner than i thought.
> There is already constant promotion function f_const_promotion() that can
> be extended to handle empty sets. So seems to me that with the current
> state of BIRD code this is simplest approach:
>
> There should be two empty sets values, one for T_SET (which behaves like
> in your patch) and one for T_PREFIX_SET (which would be just empty trie).
> Constant [] would generate the T_SET empty constant, but f_const_promotion()
> would handle promotion of T_SET empty constant to T_PREFIX_SET empty
> constant when needed. That should fix issues with variables/arguments.
>
> --
> Elen sila lumenn' omentielvo
>
> Ondrej 'Santiago' Zajicek (email: santiago at crfreenet.org)
> OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
> "To err is human -- to blame it on a computer is even more so."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bird-empty-set.patch-v2
Type: application/octet-stream
Size: 15211 bytes
Desc: not available
URL: <http://trubka.network.cz/pipermail/bird-users/attachments/20220222/64b40e3f/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bird-empty-set.patch-v1-v2
Type: application/octet-stream
Size: 8505 bytes
Desc: not available
URL: <http://trubka.network.cz/pipermail/bird-users/attachments/20220222/64b40e3f/attachment-0001.obj>