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 <santi...@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: santi...@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."
bird-empty-set.patch-v2
Description: Binary data
bird-empty-set.patch-v1-v2
Description: Binary data
