On Tue, Feb 22, 2022 at 08:18:35PM +0100, Alexander Zubkov wrote:
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.
OK. Also you could pass f_val *, so you would avoid the include. Although 3 parameters are also fine.
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.
One solution would be a global constant empty trie, so you can just set pointer to that. Alternative would be just have NULL like in T_SET, but that would mean to add several special cases to trie functions, so that is probably worse approach. The difference is that for that trees NULLs are naturally valid values, while tries have header.
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.
Yes it is a copy stored in FI_CONSTANT instruction: decl.m4:488 struct f_val *c = &arg->i_FI_CONSTANT.val It is copied during parsing from the global symbol: config.Y:731 $$ = f_new_inst(FI_CONSTANT, *($1->val))
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.
Nak. That is intentional and documented. And the same behavior is for T_*LIST. Both filter(path, integer) and filter(clist, pair) is pretty useless and would require extending existing *list-handling code just to do that.
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.
That is oversight on my side, there should be static typechecks that also do promotions. Will look at that.
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.
Yes, if FI_EQ used ARG_SAME_TYPE(), the typecheck would force promotion, but our equality check accepts any pair of objects (and returns false for object of different types instead of failing). I will look into this by implementing some restricted variant of ARG_SAME_TYPE() that forces promotion but otherwise ignores mismatched types. (although one could argue that if we already know in parse time that comparison would fail due to different types, then something is wrong.)
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.
-- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santiago@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."