move peephole optimization to xlat_purify_op()

diff --git a/src/lib/unlang/xlat.h b/src/lib/unlang/xlat.h
index a2e91faa7631efde4f1bba0c36455adfc32e2f97..44052de623ec90b6c7c4d7caad18a91615a2427f 100644 (file)
--- a/src/lib/unlang/xlat.h
+++ b/src/lib/unlang/xlat.h
@@ -417,6 +417,8 @@ void                xlat_instances_free(void);
 typedef struct unlang_interpret_s unlang_interpret_t;
 int            xlat_purify(xlat_exp_head_t *head, unlang_interpret_t *intp);
 
+int            xlat_purify_op(TALLOC_CTX *ctx, xlat_exp_t **out, xlat_exp_t *lhs, fr_token_t op, xlat_exp_t *rhs);
+
 /*
  *     xlat.c
  */

diff --git a/src/lib/unlang/xlat_expr.c b/src/lib/unlang/xlat_expr.c
index 331a2aafb190d0b09b4f50b63586451a4e6a5eb2..e8e99ddad296090f4160c3fa69a7b79c591cc9b7 100644 (file)
--- a/src/lib/unlang/xlat_expr.c
+++ b/src/lib/unlang/xlat_expr.c
@@ -2461,107 +2461,6 @@ static bool valid_type(xlat_exp_t *node)
 }
 
 
-static fr_value_box_t *xlat_value_box(xlat_exp_t *node)
-{
-       if (!node) return NULL;
-
-       if (node->type == XLAT_BOX) {
-               return &node->data;
-
-       } else if ((node->type == XLAT_TMPL) && tmpl_is_data(node->vpt)) {
-               return tmpl_value(node->vpt);
-       }
-
-       return NULL;
-}
-
-
-static bool is_truthy(xlat_exp_t *node, bool *out)
-{
-       fr_value_box_t const *box;
-
-       box = xlat_value_box(node);
-       if (!box) {
-               *out = false;
-               return false;
-       }
-
-       *out = fr_value_box_is_truthy(box);
-       return true;
-}
-
-/*
- *     Do some optimizations.
- *
- *     @todo - check for tail of LHS
- *
- *     if ((lhs->type == XLAT_FUNC) && (lhs->call.func->token == op))
- *             && tail is truthy, then remove tail, replace it with RHS
- *             and return LHS.
- *
- *     lhs->call.args->flags.can_purify |= rhs->flags.can_purify | rhs->flags.pure;
- *     lhs->flags.can_purify = lhs->call.args->flags.can_purify;
- */
-static xlat_exp_t *logical_peephole_optimize(xlat_exp_t *lhs, fr_token_t op, xlat_exp_t *rhs)
-{
-       bool value;
-
-       if (!is_truthy(lhs, &value)) return NULL;
-
-       /*
-        *      1 && FOO   --> FOO
-        *      0 && FOO   --> 0
-        *      FOO && BAR --> FOO && BAR
-        */
-
-       /*
-        *      1 || FOO   --> 1
-        *      0 || FOO   --> FOO
-        *      FOO || BAR --> FOO || BAR
-        */
-       if (value == (op != T_LAND)) {
-               talloc_free(rhs);
-               return lhs;
-       }
-       
-       talloc_free(lhs);
-       return rhs;
-}
-
-
-/*
- *     Do some optimizations
- */
-static int binary_peephole_optimize(TALLOC_CTX *ctx, xlat_exp_t **out, xlat_exp_t *lhs, fr_token_t op, xlat_exp_t *rhs)
-{
-       fr_value_box_t *lhs_box, *rhs_box;
-       fr_value_box_t box;
-       xlat_exp_t *node;
-       char *name;
-
-       lhs_box = xlat_value_box(lhs);
-       if (!lhs_box) return 0;
-
-       rhs_box = xlat_value_box(rhs);
-       if (!rhs_box) return 0;
-
-       if (fr_value_calc_binary_op(lhs, &box, FR_TYPE_NULL, lhs_box, op, rhs_box) < 0) return -1;
-
-       MEM(node = xlat_exp_alloc_null(ctx));
-       xlat_exp_set_type(node, XLAT_BOX);
-
-       if (box.type == FR_TYPE_BOOL) box.enumv = attr_expr_bool_enum;
-
-       (void) fr_value_box_aprint(node, &name, &box, NULL);
-
-       xlat_exp_set_name_buffer_shallow(node, name);
-       fr_value_box_copy(node, &node->data, &box);
-
-       *out = node;
-
-       return 1;
-}
-
 /** Tokenize a mathematical operation.
  *
  *     (EXPR)
@@ -2734,8 +2633,8 @@ redo:
        fr_assert(func != NULL);
 
        /*
-        *      If it's a logical operator, check for rcodes, and also
-        *      merge sequences of the same operator together.
+        *      If it's a logical operator, check for rcodes, and then
+        *      try to purify the results.
         */
        if (logical_ops[op]) {
                if (reparse_rcode(head, &rhs, true) < 0) {
@@ -2753,23 +2652,11 @@ redo:
                }
 
                if (reparse_rcode(head, &lhs, true) < 0) goto fail_lhs;
-
-               /*
-                *      Peephole optimizer.
-                *
-                *              FOO || 0 --> FOO
-                *              FOO && 1 --> 1
-                */
-               node = logical_peephole_optimize(lhs, op, rhs);
-               if (node) {
-               replace:
-                       lhs = node;
-                       goto redo;
-               }
+               goto purify;
        }
 
        /*
-        *      Remove invalid comparisons.
+        *      Complain on comparisons between invalid data types.
         *
         *      @todo - allow
         *
@@ -2790,10 +2677,14 @@ redo:
                if (cond) {
                        int rcode;
 
-                       rcode = binary_peephole_optimize(head, &node, lhs, op, rhs);
+               purify:
+                       rcode = xlat_purify_op(head, &node, lhs, op, rhs);
                        if (rcode < 0) goto fail_lhs;
 
-                       if (rcode) goto replace;
+                       if (rcode) {
+                               lhs = node;
+                               goto redo;
+                       }
                }
        }
 

diff --git a/src/lib/unlang/xlat_purify.c b/src/lib/unlang/xlat_purify.c
index 077802a328ac87b37f681b76d61246749db80c7c..edf5e69cd156fc105dfd31394cecbd19214ee202 100644 (file)
--- a/src/lib/unlang/xlat_purify.c
+++ b/src/lib/unlang/xlat_purify.c
@@ -28,6 +28,7 @@ RCSID("$Id$")
 
 #include <freeradius-devel/server/base.h>
 #include <freeradius-devel/unlang/xlat_priv.h>
+#include <freeradius-devel/util/calc.h>
 
 static void xlat_value_list_to_xlat(xlat_exp_head_t *head, fr_value_box_list_t *list)
 {
@@ -211,3 +212,125 @@ int xlat_purify(xlat_exp_head_t *head, unlang_interpret_t *intp)
 
        return rcode;
 }
+
+static fr_value_box_t *xlat_value_box(xlat_exp_t *node)
+{
+#ifdef STATIC_ANALYZER
+       if (!node) return NULL;
+#endif
+
+       if (node->type == XLAT_BOX) {
+               return &node->data;
+
+       } else if ((node->type == XLAT_TMPL) && tmpl_is_data(node->vpt)) {
+               return tmpl_value(node->vpt);
+       }
+
+       return NULL;
+}
+
+
+static bool is_truthy(xlat_exp_t *node, bool *out)
+{
+       fr_value_box_t const *box;
+
+       box = xlat_value_box(node);
+       if (!box) {
+               *out = false;
+               return false;
+       }
+
+       *out = fr_value_box_is_truthy(box);
+       return true;
+}
+
+/*
+ *     Do some optimizations.
+ *
+ */
+static xlat_exp_t *logical_peephole_optimize(xlat_exp_t *lhs, fr_token_t op, xlat_exp_t *rhs)
+{
+       bool value;
+
+       /*
+        *      @todo - check for tail of LHS
+        *              && tail is truthy, then remove tail, and call ourselves recursively
+        *              if there's a new node, it becomes the new tail.  Otherwise
+        *              we append the rhs to the lhs args.
+        *
+        *      lhs->call.args->flags.can_purify |= rhs->flags.can_purify | rhs->flags.pure;
+        *      lhs->flags.can_purify = lhs->call.args->flags.can_purify;
+        */
+       if (!is_truthy(lhs, &value)) return NULL;
+
+       /*
+        *      1 && FOO   --> FOO
+        *      0 && FOO   --> 0
+        *      FOO && BAR --> FOO && BAR
+        */
+
+       /*
+        *      1 || FOO   --> 1
+        *      0 || FOO   --> FOO
+        *      FOO || BAR --> FOO || BAR
+        */
+       if (value == (op != T_LAND)) {
+               talloc_free(rhs);
+               return lhs;
+       }
+
+       talloc_free(lhs);
+       return rhs;
+}
+
+
+/*
+ *     Do some optimizations
+ *
+ *     @todo check types, if one side is uint8, and the other side is uint32, there are some situations where
+ *     the comparison will always fail.  And should therefore be invalid?
+ */
+static int binary_peephole_optimize(TALLOC_CTX *ctx, xlat_exp_t **out, xlat_exp_t *lhs, fr_token_t op, xlat_exp_t *rhs)
+{
+       fr_value_box_t *lhs_box, *rhs_box;
+       fr_value_box_t box;
+       xlat_exp_t *node;
+       char *name;
+
+       lhs_box = xlat_value_box(lhs);
+       if (!lhs_box) return 0;
+
+       rhs_box = xlat_value_box(rhs);
+       if (!rhs_box) return 0;
+
+       if (fr_value_calc_binary_op(lhs, &box, FR_TYPE_NULL, lhs_box, op, rhs_box) < 0) return -1;
+
+       MEM(node = xlat_exp_alloc_null(ctx));
+       xlat_exp_set_type(node, XLAT_BOX);
+
+       if (box.type == FR_TYPE_BOOL) box.enumv = attr_expr_bool_enum;
+
+       (void) fr_value_box_aprint(node, &name, &box, NULL);
+
+       xlat_exp_set_name_buffer_shallow(node, name);
+       fr_value_box_copy(node, &node->data, &box);
+
+       *out = node;
+
+       return 1;
+}
+
+int xlat_purify_op(TALLOC_CTX *ctx, xlat_exp_t **out, xlat_exp_t *lhs, fr_token_t op, xlat_exp_t *rhs)
+{
+       if ((op == T_LAND) || (op == T_LOR)) {
+               xlat_exp_t *node;
+
+               node = logical_peephole_optimize(lhs, op, rhs);
+               if (!node) return 0;
+
+               *out = node;
+               return 1;
+       }
+
+       return binary_peephole_optimize(ctx, out, lhs, op, rhs);
+}