Filter: Implement mixed declarations of local variables

Allow variable declarations mixed with code, also in nested blocks with
proper scoping, and with variable initializers. E.g:

function fn(int a)
{
  int b;
  int c = 10;

  if a > 20 then
  {
    b = 30;
    int d = c * 2;
    print a, b, c, d;
  }

  string s = "Hello";
}

4 files changed, 116 insertions, 37 deletions

diff --git a/doc/bird.sgml b/doc/bird.sgml
index 326fc7a8..f933128c 100644
--- a/doc/bird.sgml
+++ b/doc/bird.sgml
@@ -1260,8 +1260,8 @@ this:
 
 <code>
 filter not_too_far
-int var;
 {
+	int var;
 	if defined( rip_metric ) then
 		var = rip_metric;
 	else {
@@ -1290,9 +1290,9 @@ local variables. Recursion is not allowed. Function definitions look like this:
 
 <code>
 function name ()
-int local_variable;
 {
-	local_variable = 5;
+	int local_variable;
+	int another_variable = 5;
 }
 
 function with_parameters (int parameter)
@@ -1301,16 +1301,19 @@ function with_parameters (int parameter)
 }
 </code>
 
-<p>Unlike in C, variables are declared after the <cf/function/ line, but before
-the first <cf/{/. You can't declare variables in nested blocks. Functions are
-called like in C: <cf>name(); with_parameters(5);</cf>. Function may return
-values using the <cf>return <m/[expr]/</cf> command. Returning a value exits
-from current function (this is similar to C).
+<p>Like in C programming language, variables are declared inside function body,
+either at the beginning, or mixed with other statements. Declarations may
+contain initialization. You can also declare variables in nested blocks, such
+variables have scope restricted to such block. There is a deprecated syntax to
+declare variables after the <cf/function/ line, but before the first <cf/{/.
+Functions are called like in C: <cf>name(); with_parameters(5);</cf>. Function
+may return values using the <cf>return <m/[expr]/</cf> command. Returning a
+value exits from current function (this is similar to C).
 
-<p>Filters are defined in a way similar to functions except they can't have
+<p>Filters are defined in a way similar to functions except they cannot have
 explicit parameters. They get a route table entry as an implicit parameter, it
 is also passed automatically to any functions called. The filter must terminate
-with either <cf/accept/ or <cf/reject/ statement. If there's a runtime error in
+with either <cf/accept/ or <cf/reject/ statement. If there is a runtime error in
 filter, the route is rejected.
 
 <p>A nice trick to debug filters is to use <cf>show route filter <m/name/</cf>
diff --git a/filter/config.Y b/filter/config.Y
index b67ca925..f8f47862 100644
--- a/filter/config.Y
+++ b/filter/config.Y
@@ -22,6 +22,36 @@ static inline u32 pair_b(u32 p) { return p & 0xFFFF; }
 #define f_generate_complex(fi_code, da, arg) \
   f_new_inst(FI_EA_SET, f_new_inst(fi_code, f_new_inst(FI_EA_GET, da), arg), da)
 
+static int
+f_new_var(struct sym_scope *s)
+{
+  /*
+   * - A variable is an offset on vstack from vbase.
+   * - Vbase is set on filter start / function call.
+   * - Scopes contain anonymous scopes (blocks) inside filter/function scope
+   * - Each scope knows number of vars in that scope
+   * - Offset is therefore a sum of 'slots' up to named scope
+   * - New variables are added on top of vstk, so intermediate values cannot
+   *   be there during FI_VAR_INIT. I.e. no 'var' inside 'term'.
+   * - Also, each f_line must always have its scope, otherwise a variable may
+   *   be defined but not initialized if relevant f_line is not executed.
+   */
+
+  int offset = s->slots++;
+
+  while (!s->name)
+  {
+    s = s->next;
+    ASSERT(s);
+    offset += s->slots;
+  }
+
+  if (offset >= 0xff)
+    cf_error("Too many variables, at most 255 allowed");
+
+  return offset;
+}
+
 /*
  * Sets and their items are during parsing handled as lists, linked
  * through left ptr. The first item in a list also contains a pointer
@@ -296,7 +326,7 @@ CF_KEYWORDS(FUNCTION, PRINT, PRINTN, UNSET, RETURN,
 %nonassoc ELSE
 
 %type <xp> cmds_int cmd_prep
-%type <x> term cmd cmds constant constructor print_list var_list function_call symbol_value bgp_path_expr bgp_path bgp_path_tail
+%type <x> term cmd cmd_var cmds cmds_scoped constant constructor print_list var var_init var_list function_call symbol_value bgp_path_expr bgp_path bgp_path_tail
 %type <fda> dynamic_attr
 %type <fsa> static_attr
 %type <f> filter where_filter
@@ -425,7 +455,7 @@ function_args:
 function_vars:
    /* EMPTY */ { $$ = 0; }
  | function_vars type symbol ';' {
-     cf_define_symbol($3, SYM_VARIABLE | $2, offset, sym_->scope->slots++);
+     cf_define_symbol($3, SYM_VARIABLE | $2, offset, f_new_var(sym_->scope));
      $$ = $1 + 1;
    }
  ;
@@ -492,7 +522,11 @@ cmds: /* EMPTY */ { $$ = NULL; }
  | cmds_int { $$ = $1.begin; }
  ;
 
-cmd_prep: cmd {
+cmds_scoped: { cf_push_soft_scope(); } cmds { cf_pop_soft_scope(); $$ = $2; } ;
+
+cmd_var: var | cmd ;
+
+cmd_prep: cmd_var {
   $$.begin = $$.end = $1;
   if ($1)
     while ($$.end->next)
@@ -639,7 +673,7 @@ fprefix_set:
  ;
 
 switch_body: /* EMPTY */ { $$ = NULL; }
- | switch_body switch_items ':' cmds  {
+ | switch_body switch_items ':' cmds_scoped  {
      /* Fill data fields */
      struct f_tree *t;
      struct f_line *line = f_linearize($4, 0);
@@ -647,7 +681,7 @@ switch_body: /* EMPTY */ { $$ = NULL; }
        t->data = line;
      $$ = f_merge_items($1, $2);
    }
- | switch_body ELSECOL cmds {
+ | switch_body ELSECOL cmds_scoped {
      struct f_tree *t = f_new_tree();
      t->from.type = t->to.type = T_VOID;
      t->right = t;
@@ -854,8 +888,19 @@ print_list: /* EMPTY */ { $$ = NULL; }
    }
  ;
 
+var_init:
+   /* empty */ { $$ = f_new_inst(FI_CONSTANT, (struct f_val) { }); }
+ | '=' term { $$ = $2; }
+ ;
+
+var:
+   type symbol var_init ';' {
+     struct symbol *sym = cf_define_symbol($2, SYM_VARIABLE | $1, offset, f_new_var(sym_->scope));
+     $$ = f_new_inst(FI_VAR_INIT, $3, sym);
+   }
+
 cmd:
-   '{' cmds '}' {
+   '{' cmds_scoped '}' {
      $$ = $2;
    }
  | IF term THEN cmd {
@@ -912,7 +957,7 @@ cmd:
  | PRINTN print_list ';' {
     $$ = f_new_inst(FI_PRINT, $2);
    }
- | function_call ';' { $$ = f_new_inst(FI_DROP_RESULT, $1); } 
+ | function_call ';' { $$ = f_new_inst(FI_DROP_RESULT, $1); }
  | CASE term '{' switch_body '}' {
       $$ = f_new_inst(FI_SWITCH, $2, build_tree($4));
    }
diff --git a/filter/f-inst.c b/filter/f-inst.c
index 5b8310c3..dc243b4e 100644
--- a/filter/f-inst.c
+++ b/filter/f-inst.c
@@ -501,6 +501,18 @@
     RESULT(T_BOOL, i, ipa_is_ip4(v1.val.ip));
   }
 
+  INST(FI_VAR_INIT, 1, 0) {
+    NEVER_CONSTANT;
+    ARG_ANY(1);
+    SYMBOL;
+    ARG_TYPE(1, sym->class & 0xff);
+
+    /* New variable is always the last on stack */
+    uint pos = curline.vbase + sym->offset;
+    fstk->vstk[pos] = v1;
+    fstk->vcnt = pos + 1;
+  }
+
   /* Set to indirect value prepared in v1 */
   INST(FI_VAR_SET, 1, 0) {
     NEVER_CONSTANT;
diff --git a/filter/test.conf b/filter/test.conf
index e8b6a663..436031a3 100644
--- a/filter/test.conf
+++ b/filter/test.conf
@@ -44,9 +44,8 @@ bt_test_same(onef, twof, 0);
  */
 
 function t_bool()
-bool b;
 {
-	b = true;
+	bool b = true;
 	bt_assert(b);
 	bt_assert(!!b);
 
@@ -82,12 +81,11 @@ define xyzzy = (120+10);
 define '1a-a1' = (xyzzy-100);
 
 function t_int()
-int i;
 {
 	bt_assert(xyzzy = 130);
 	bt_assert('1a-a1' = 30);
 
-	i = four;
+	int i = four;
 	i = 12*100 + 60/2 + i;
 	i = (i + 0);
 	bt_assert(i = 1234);
@@ -128,9 +126,8 @@ define is2 = [(17+2), 17, 15, 11, 8, 5, 3, 2];
 define is3 = [5, 17, 2, 11, 8, 15, 3, 19];
 
 function t_int_set()
-int set is;
 {
-	is = [];
+	int set is = [];
 	bt_assert(is = []);
 	bt_assert(0 !~ is);
 
@@ -190,9 +187,8 @@ bt_test_suite(t_int_set, "Testing sets of integers");
  */
 
 function t_string()
-string st;
 {
-	st = "Hello";
+	string st = "Hello";
 	bt_assert(format(st) = "Hello");
 	bt_assert(st ~ "Hell*");
 	bt_assert(st ~ "?ello");
@@ -217,9 +213,8 @@ function 'mkpair-a'(int a)
 }
 
 function t_pair()
-pair pp;
 {
-	pp = (1, 2);
+	pair pp = (1, 2);
 	bt_assert(format(pp) = "(1,2)");
 	bt_assert((1,2) = pp);
 	bt_assert((1,1+1) = pp);
@@ -240,11 +235,9 @@ bt_test_suite(t_pair, "Testing pairs");
  */
 
 function t_pair_set()
-pair pp;
-pair set ps;
 {
-	pp = (1, 2);
-	ps = [];
+	pair pp = (1, 2);
+	pair set ps = [];
 	bt_assert(pp !~ ps);
 
 	ps = [(1,(one+one)), (3,4)..(4,8), (5,*), (6,3..6)];
@@ -1290,6 +1283,34 @@ function fifteen()
 	return 15;
 }
 
+function local_vars(int j)
+{
+	int k = 10;
+	bt_assert(j = 5 && k = 10);
+	{
+		int j = 15;
+		k = 20;
+		bt_assert(j = 15 && k = 20);
+	}
+	bt_assert(j = 5 && k = 20);
+
+	if j < 10 then
+	{
+		int j = 25;
+		string k = "hello";
+		bt_assert(j = 25 && k = "hello");
+	}
+	bt_assert(j = 5 && k = 20);
+
+	int m = 100;
+	{
+		j = 35;
+		int k = 40;
+		bt_assert(j = 35 && k = 40 && m = 100);
+	}
+	bt_assert(j = 35 && k = 20 && m = 100);
+}
+
 function factorial(int x)
 {
 	if x = 0 then return 0;
@@ -1312,21 +1333,18 @@ function hanoi_init(int a; int b)
 }
 
 function hanoi_solve(int n; bgppath h_src; bgppath h_dst; bgppath h_aux; bool x; bool y)
-bgppath tmp1;
-bgppath tmp2;
-int v;
 {
 	# x -> return src or dst
 	# y -> print state
 
 	if n = 0 then { if x then return h_src; else return h_dst; }
 
-	tmp1 = hanoi_solve(n - 1, h_src, h_aux, h_dst, true, y);
-	tmp2 = hanoi_solve(n - 1, h_src, h_aux, h_dst, false, false);
+	bgppath tmp1 = hanoi_solve(n - 1, h_src, h_aux, h_dst, true, y);
+	bgppath tmp2 = hanoi_solve(n - 1, h_src, h_aux, h_dst, false, false);
 	h_src = tmp1;
 	h_aux = tmp2;
 
-	v = h_src.first;
+	int v = h_src.first;
 	# bt_assert(h_dst = +empty+ || v < h_dst.first);
 	h_src = delete(h_src, v);
 	h_dst = prepend(h_dst, v);
@@ -1355,6 +1373,7 @@ bgppath h_src;
 	bt_assert(callme(4, 4) = 16);
 	bt_assert(callme(7, 2) = 14);
 	bt_assert(callmeagain(1, 2, 3) = 6);
+	local_vars(5);
 
 	bt_assert(factorial(5) = 120);
 	bt_assert(factorial(10) = 3628800);