From 39e75b879b1464f35725bb218d9e14399fc4c291 Mon Sep 17 00:00:00 2001
From: Ondrej Zajicek <santiago@crfreenet.org>
Date: Tue, 8 Oct 2024 19:34:51 +0200
Subject: [PATCH] Filter: Fixes and improvements related to case/sets

Unify grammar for set_atom and switch_atom to avoid inconsistencies
between them. Fix errors in documentation related to case statement
and set type. Change 'vpnrd' to 'rd' to be consistent with the filter
language.

Thanks to Mikhail Mayorov for bugreport.
---
 doc/bird.sgml    | 43 ++++++++++++++++++++++++-------------------
 filter/config.Y  | 21 ++++++++++++---------
 filter/test.conf |  2 +-
 3 files changed, 37 insertions(+), 29 deletions(-)

diff --git a/doc/bird.sgml b/doc/bird.sgml
index b0f130f1..e04f8d0e 100644
--- a/doc/bird.sgml
+++ b/doc/bird.sgml
@@ -1650,7 +1650,7 @@ in the foot).
 	Route Distinguisher (<rfc id="4364">). They support the same special
 	operators as IP prefixes, and also <cf/.rd/ which extracts the Route
 	Distinguisher. Their literals are written
-	as <cf><m/vpnrd/ <m/ipprefix/</cf>
+	as <cf><m/rd/ <m/ipprefix/</cf>
 
 	<cf/NET_ROA4/ and <cf/NET_ROA6/ prefixes hold an IP prefix range
 	together with an ASN. They support the same special operators as IP
@@ -1665,9 +1665,9 @@ in the foot).
 	<cf/NET_MPLS/ holds a single MPLS label and its handling is currently
 	not implemented.
 
-	<tag><label id="type-vpnrd">vpnrd</tag>
+	<tag><label id="type-rd"><label id="type-vpnrd">rd</tag>
 	This is a route distinguisher according to <rfc id="4364">. There are
-	three kinds of RD's: <cf><m/asn/:<m/32bit int/</cf>, <cf><m/asn4/:<m/16bit int/</cf>
+	three kinds of RDs: <cf><m/asn/:<m/32bit int/</cf>, <cf><m/asn4/:<m/16bit int/</cf>
 	and <cf><m/IPv4 address/:<m/32bit int/</cf>
 
 	<tag><label id="type-ec">ec</tag>
@@ -1694,9 +1694,9 @@ in the foot).
 	to extract corresponding components of LCs:
 	<cf>(<m/asn/, <m/data1/, <m/data2/)</cf>.
 
-	<tag><label id="type-set">int|pair|quad|ip|prefix|ec|lc|enum set</tag>
-	Filters recognize four types of sets. Sets are similar to strings: you
-	can pass them around but you can't modify them. Literals of type <cf>int
+	<tag><label id="type-set">int|pair|quad|ip|prefix|ec|lc|rd|enum set</tag>
+	Filters recognize several types of sets. Sets are similar to strings: you
+	can pass them around but you cannot modify them. Literals of type <cf>int
 	set</cf> look like <cf> [ 1, 2, 5..7 ]</cf>. As you can see, both simple
 	values and ranges are permitted in sets.
 
@@ -1719,9 +1719,11 @@ in the foot).
 	is valid, while <cf/(10, *, 20..30)/ or <cf/(10, 20..30, 40)/ is not
 	valid.
 
-	You can also use expressions for int, pair, EC and LC set values.
-	However, it must be possible to evaluate these expressions before daemon
-	boots. So you can use only constants inside them. E.g.
+	You can also use named constants or compound expressions for non-prefix
+	set values. However, it must be possible to evaluate these expressions
+	before daemon boots. So you can use only constants inside them. Also,
+	in case of compound expressions, they require parentheses around them.
+	E.g.
 
 	<code>
 	 define one=1;
@@ -1730,7 +1732,7 @@ in the foot).
 	 pair set ps;
 	 ec set es;
 
-	 odds = [ one, 2+1, 6-one, 2*2*2-1, 9, 11 ];
+	 odds = [ one, (2+1), (6-one), (2*2*2-1), 9, 11 ];
 	 ps = [ (1,one+one), (3,4)..(4,8), (5,*), (6,3..6), (7..9,*) ];
 	 es = [ (rt, myas, 3*10), (rt, myas+one, 0..16*16*16-1), (ro, myas+2, *) ];
 	</code>
@@ -1943,13 +1945,16 @@ may be an existing one (when just name is used) or a locally defined (when type
 and name is used). In both cases, it must have the same type as elements.
 
 <p>The <cf>case</cf> is similar to case from Pascal. Syntax is <cf>case
-<m/expr/ { else: | <m/num_or_prefix [ .. num_or_prefix]/: <m/statement/ ; [
-... ] }</cf>. The expression after <cf>case</cf> can be of any type which can be
-on the left side of the &tilde; operator and anything that could be a member of
-a set is allowed before <cf/:/. Multiple commands are allowed without <cf/{}/
-grouping. If <cf><m/expr/</cf> matches one of the <cf/:/ clauses, statements
-between it and next <cf/:/ statement are executed. If <cf><m/expr/</cf> matches
-neither of the <cf/:/ clauses, the statements after <cf/else:/ are executed.
+<m/expr/ { else: | <m/set_body_expr/ /: <m/statement/ ; [... ] }</cf>.
+
+The expression after <cf>case</cf> can be of any type that could be a member of
+a set, while the <m/set_body_expr/ before <cf/:/ can be anything (constants,
+intervals, expressions) that could be a part of a set literal. One exception is
+prefix type, which can be used in sets bud not in <cf/case/ structure. Multiple
+commands are allowed without <cf/{}/ grouping. If <cf><m/expr/</cf> matches one
+of the <cf/:/ clauses, statements between it and next <cf/:/ statement are
+executed. If <cf><m/expr/</cf> matches neither of the <cf/:/ clauses, the
+statements after <cf/else:/ are executed.
 
 <p>Here is example that uses <cf/if/ and <cf/case/ structures:
 
@@ -4085,11 +4090,11 @@ could have up to 5 channels: <cf/ipv4/, <cf/ipv6/, <cf/vpn4/, <cf/vpn6/, and
 <cf/mpls/.
 
 <p><descrip>
-	<tag><label id="l3vpn-route-distinguisher">route distinguisher <m/vpnrd/</tag>
+	<tag><label id="l3vpn-route-distinguisher">route distinguisher <m/rd/</tag>
 	The route distinguisher that is attached to routes in the export
 	direction. Mandatory.
 
-	<tag><label id="l3vpn-rd">rd <m/vpnrd/</tag>
+	<tag><label id="l3vpn-rd">rd <m/rd/</tag>
 	A shorthand for the option <cf/route distinguisher/.
 
 	<tag><label id="l3vpn-import-target">import target <m/ec/|<m/ec-set/</tag>
diff --git a/filter/config.Y b/filter/config.Y
index 5c3a1c16..d656a726 100644
--- a/filter/config.Y
+++ b/filter/config.Y
@@ -390,7 +390,7 @@ CF_KEYWORDS(FUNCTION, PRINT, PRINTN, UNSET, RETURN,
 %type <i32> cnum
 %type <e> pair_item ec_item lc_item set_item switch_item ec_items set_items switch_items switch_body
 %type <trie> fprefix_set
-%type <v> set_atom switch_atom fipa
+%type <v> set_atom0 set_atom switch_atom fipa
 %type <px> fprefix
 %type <t> get_cf_position
 %type <s> for_var
@@ -636,27 +636,30 @@ fipa:
  * as a function call in switch case cmds.
  */
 
-set_atom:
+set_atom0:
    NUM    { $$.type = T_INT; $$.val.i = $1; }
  | fipa   { $$ = $1; }
  | VPN_RD { $$.type = T_RD; $$.val.ec = $1; }
  | ENUM   { $$.type = pair_a($1); $$.val.i = pair_b($1); }
  | '(' term ')' {
      $$ = cf_eval($2, T_VOID);
-     if (!f_valid_set_type($$.type)) cf_error("Set-incompatible type");
+     if (!f_valid_set_type($$.type))
+       cf_error("Set-incompatible type (%s)", f_type_name($$.type));
    }
+ ;
+
+set_atom:
+   set_atom0
  | symbol_known {
      cf_assert_symbol($1, SYM_CONSTANT);
-     if (!f_valid_set_type(SYM_TYPE($1))) cf_error("%s: set-incompatible type", $1->name);
+     if (!f_valid_set_type(SYM_TYPE($1)))
+       cf_error("%s: Set-incompatible type (%s)", $1->name, f_type_name(SYM_TYPE($1)));
      $$ = *$1->val;
    }
  ;
 
 switch_atom:
-   NUM   { $$.type = T_INT; $$.val.i = $1; }
- | '(' term ')' { $$ = cf_eval($2, T_INT); }
- | fipa  { $$ = $1; }
- | ENUM  { $$.type = pair_a($1); $$.val.i = pair_b($1); }
+   set_atom0
  ;
 
 cnum:
@@ -749,7 +752,7 @@ fprefix_set:
  ;
 
 switch_body: /* EMPTY */ { $$ = NULL; }
- | switch_body switch_items ':' cmds_scoped  {
+ | switch_body switch_items ':' cmds_scoped {
      /* Fill data fields */
      struct f_tree *t;
      for (t = $2; t; t = t->left)
diff --git a/filter/test.conf b/filter/test.conf
index 02857eac..0361a1c9 100644
--- a/filter/test.conf
+++ b/filter/test.conf
@@ -138,7 +138,7 @@ function t_int()
 	}
 
 	case four {
-		4: bt_assert(true);
+		(2+2): bt_assert(true);
 		else: bt_assert(false);
 	}