make cmp return bool and fix associated tests

diff --git a/src/lib/unlang/xlat_expr.c b/src/lib/unlang/xlat_expr.c
index a51061f210fb8d1aec811f7d14c87a1dc90ec277..a384ba2b976daeec384065237a46e81bb7eb235f 100644 (file)
--- a/src/lib/unlang/xlat_expr.c
+++ b/src/lib/unlang/xlat_expr.c
@@ -471,8 +471,6 @@ static xlat_action_t xlat_cmp_op(TALLOC_CTX *ctx, fr_dcursor_t *out,
        int rcode;
        fr_value_box_t  *dst, *a, *b;
 
-       MEM(dst = fr_value_box_alloc_null(ctx));
-
        /*
         *      Each argument is a FR_TYPE_GROUP, with one or more elements in a list.
         */
@@ -486,16 +484,18 @@ static xlat_action_t xlat_cmp_op(TALLOC_CTX *ctx, fr_dcursor_t *out,
 
        fr_assert(fr_comparison_op[op]);
 
+       MEM(dst = fr_value_box_alloc(ctx, FR_TYPE_BOOL, attr_expr_bool_enum));
+
        rcode = fr_value_calc_list_cmp(dst, dst, &a->vb_group, op, &b->vb_group);
        if (rcode < 0) {
-               RPEDEBUG("Failed calculating result, returning NULL");
-               goto done;
+               talloc_free(dst);
+               RPEDEBUG("Failed calculating result, returning fail");
+               return XLAT_ACTION_FAIL;
        }
 
        fr_assert(dst->type == FR_TYPE_BOOL);
        dst->enumv = attr_expr_bool_enum;
 
-done:
        fr_dcursor_append(out, dst);
        VALUE_BOX_LIST_VERIFY((fr_value_box_list_t *)out->dlist);
        return XLAT_ACTION_DONE;
@@ -1666,7 +1666,7 @@ do { \
 #undef XLAT_REGISTER_BINARY_CMP
 #define XLAT_REGISTER_BINARY_CMP(_op, _name) \
 do { \
-       if (unlikely((xlat = xlat_func_register(NULL, "cmp_" STRINGIFY(_name), xlat_func_cmp_ ## _name, FR_TYPE_VOID)) == NULL)) return -1; \
+       if (unlikely((xlat = xlat_func_register(NULL, "cmp_" STRINGIFY(_name), xlat_func_cmp_ ## _name, FR_TYPE_BOOL)) == NULL)) return -1; \
        xlat_func_args_set(xlat, binary_cmp_xlat_args); \
        xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_PURE | XLAT_FUNC_FLAG_INTERNAL); \
        xlat_func_print_set(xlat, xlat_expr_print_binary); \

diff --git a/src/lib/unlang/xlat_purify.c b/src/lib/unlang/xlat_purify.c
index 82f242cf24efda68f59bcbf9f43c00b37ff0bca8..fd297fe6fb0273e214bfd380598b7ac0a111b3fb 100644 (file)
--- a/src/lib/unlang/xlat_purify.c
+++ b/src/lib/unlang/xlat_purify.c
@@ -255,10 +255,25 @@ static xlat_exp_t *peephole_optimize_lor(xlat_exp_t *lhs,  xlat_exp_t *rhs)
                /*
                 *      FOO || 0 --> FOO much of the time
                 *      FOO || 1 --> FOO much of the time
+                */
+               if (!is_truthy(rhs, &value)) return NULL;
+
+               /*
+                *      BOOL || 1 --> 1
+                *
+                *      Because if the LHS is 1, then we return the LHS (1)
+                *      On the other hand, it the LHS is 0, then we return
+                *      the RHS, which is also 1.
                 *
-                *      @todo - if the LHS is a function AND the LHS returns a boolean, THEN we can optimized
-                *      the "FOO || 1" case to just "1".
+                *      But we can't do
+                *
+                *      <type> || 1 --> 1
                 */
+               if (value && (lhs->type == XLAT_FUNC) && (lhs->call.func->return_type == FR_TYPE_BOOL)) {
+                       talloc_free(lhs);
+                       return rhs;
+               }
+
                return NULL;
        }
 

diff --git a/src/tests/unit/condition/base.txt b/src/tests/unit/condition/base.txt
index afe19da23433b4fef163dad240b0a908f30cc24d..5a5fafc2b665a69e0a69b6289406c258be3b1440 100644 (file)
--- a/src/tests/unit/condition/base.txt
+++ b/src/tests/unit/condition/base.txt
@@ -668,7 +668,7 @@ match (&User-Name || true)
 #  can't optimize it.
 #
 condition (&User-Name == "bob") || (true)
-match ((&User-Name == "bob") || true)
+match true
 
 #
 #  A && (B || C) is not the same as (A && B) || C, for 0/1/1

diff --git a/src/tests/unit/xlat/cond_base.txt b/src/tests/unit/xlat/cond_base.txt
index 3601a39ee6cc43e23f86bcf00056038698a660ad..2a24899e86379e483ce0c3ee0221f9eea6cdc30a 100644 (file)
--- a/src/tests/unit/xlat/cond_base.txt
+++ b/src/tests/unit/xlat/cond_base.txt
@@ -110,7 +110,7 @@ xlat_purify (!"foo" == "bar")
 match ERROR offset 2: Operator '!' is only applied to the left hand side of the '==' operation, add (..) to evaluate the operation first
 
 xlat_purify ((!"foo") == "bar")
-match NULL
+match (!"foo" == "bar")
 
 xlat_purify ((!"foo") == false)
 match true
@@ -251,9 +251,9 @@ xlat_purify (ipaddr)127.0.0.1 == "127.0.0.1"
 match true
 
 # LHS is IPaddr, RHS is string (malformed IP address).
-# Condition code attempts to cast md4 hash to IP address resulting in an invalid comparison
+# We can only fail this at run-time.
 xlat_purify (ipaddr)127.0.0.1 == "%{md4: 127.0.0.1}"
-match NULL
+match (127.0.0.1 == %(cast:string "%{md4: 127.0.0.1}"))
 
 #
 #  Bare %{...} is allowed.
@@ -261,7 +261,7 @@ match NULL
 
 # Invalid cast from octets to ipaddr.
 xlat_purify (ipaddr)127.0.0.1 == %{md4:127.0.0.1}
-match NULL
+match (127.0.0.1 == %{md4:127.0.0.1})
 
 xlat_purify (ipaddr)127.0.0.1 == %{md4: SELECT user FROM table WHERE user='%{User-Name}'}
 match (127.0.0.1 == %{md4: SELECT user FROM table WHERE user='%{User-Name}'})
@@ -271,7 +271,7 @@ match true
 
 # Invalid cast from octets to ether.
 xlat_purify (ether)00:11:22:33:44:55 == "%{md4:00:11:22:33:44:55}"
-match NULL
+match (00:11:22:33:44:55 == %(cast:string "%{md4:00:11:22:33:44:55}"))
 
 xlat_purify (ether) 00:XX:22:33:44:55 == 00:11:22:33:44:55
 match ERROR offset 12: Missing separator, expected ':'