groupify arguments to functions

for now, we create the arguments and then groupify them if necessary.

The alternative is to always create grouping nodes, and then
remove / reparent them if they're not necessary.

diff --git a/src/lib/unlang/xlat_expr.c b/src/lib/unlang/xlat_expr.c
index c90e1e8ffcca2f31df5ec307bab5a236586d9fa8..f10dce7a47aee036674414e11e581a9a935d8b53 100644 (file)
--- a/src/lib/unlang/xlat_expr.c
+++ b/src/lib/unlang/xlat_expr.c
@@ -49,9 +49,12 @@ RCSID("$Id$")
  *     @todo - short-circuit && / || need to be updated.  This requires various magic in their instantiation
  *     routines, which is not yet done.
  *
- *     @todo - Check for input flags->pure, and node->internal && node->token != T_INVALID && node->pure.  If
- *     so, we call xlat_purify() to purify the results.  This capability lets us write tests for parsing
- *     which use simple numbers, to verify that the parser is OK.
+ *     @todo - all function arguments should be in groups, so we need to fix that.  Right now, binary
+ *     expressions are fixed.  But unary ones are not.  We did it via a hack, but it might be better to do it
+ *     a different way in the future.  The problem is that no matter which way we choose, we'll have to
+ *     talloc_steal() something.
+ *
+ *     @todo - run xlat_purify_expr() after creating the unary node.
  *
  *     The purify function should also be smart enough to do things like remove redundant casts.
  *
@@ -215,6 +218,52 @@ cleanup:
        return rcode;
 }
 
+static xlat_exp_t *xlat_groupify_node(TALLOC_CTX *ctx, xlat_exp_t *node)
+{
+       xlat_exp_t *group;
+
+       fr_assert(node->type != XLAT_GROUP);
+
+       group = xlat_exp_alloc_null(ctx);
+       xlat_exp_set_type(group, XLAT_GROUP);
+       group->quote = T_BARE_WORD;
+
+       group->child = talloc_steal(group, node);
+       group->flags = node->flags;
+
+       if (node->next) {
+               group->next = xlat_groupify_node(ctx, node->next);
+               node->next = NULL;
+       }
+
+       return group;
+}
+
+/*
+ *     Any function requires each argument to be in it's own XLAT_GROUP.  But we can't create an XLAT_GROUP
+ *     from the start of parsing, as we might need to return an XLAT_FUNC, or another type of xlat.  Instead,
+ *     we just work on the bare nodes, and then later groupify them.  For now, it's just easier to do it this way.
+ */
+static void xlat_groupify_expr(xlat_exp_t *node)
+{
+       xlat_t const *func;
+
+       if (node->type != XLAT_FUNC) return;
+
+       func = node->call.func;
+
+       if (!func->internal) return;
+
+       if (func->token == T_INVALID) return;
+
+       /*
+        *      It's already been groupified, don't do anything.
+        */
+       if (node->child->type == XLAT_GROUP) return;
+
+       node->child = xlat_groupify_node(node, node->child);
+}
+
 static xlat_arg_parser_t const cast_xlat_args[] = {
        { .required = true, .type = FR_TYPE_INT32 },
        { .required = true, .type = FR_TYPE_VOID },
@@ -948,6 +997,8 @@ done:
                cast->child->next = node;
                xlat_flags_merge(&cast->flags, &node->flags);
                node = cast;
+
+               xlat_groupify_expr(node);
        }
 
 check_unary:
@@ -1189,6 +1240,11 @@ redo:
                }
        }
 
+       /*
+        *      Ensure that the various nodes are grouped properly.
+        */
+       xlat_groupify_expr(node);
+
        lhs = node;
        goto redo;
 }

diff --git a/src/lib/unlang/xlat_tokenize.c b/src/lib/unlang/xlat_tokenize.c
index bd2d00ffa0a3833385af60907aa1a3451f8c2875..443b1b009307a2b81263fbe1dd798c8a0252abe4 100644 (file)
--- a/src/lib/unlang/xlat_tokenize.c
+++ b/src/lib/unlang/xlat_tokenize.c
@@ -1136,10 +1136,13 @@ static ssize_t xlat_print_node(fr_sbuff_t *out, xlat_exp_t const *head, fr_sbuff
                if (node->quote != T_BARE_WORD) FR_SBUFF_IN_CHAR_RETURN(out, fr_token_quote[node->quote]);
                xlat_print(out, node->child, fr_value_escape_by_quote[node->quote]);
                if (node->quote != T_BARE_WORD) FR_SBUFF_IN_CHAR_RETURN(out, fr_token_quote[node->quote]);
-               if (node->next) FR_SBUFF_IN_CHAR_RETURN(out, ' ');      /* Add ' ' between args */
+               if (node->next) FR_SBUFF_IN_CHAR_RETURN(out, ' ');      /* Add ' ' between args */
                goto done;
 
        case XLAT_BOX:
+               /*
+                *      @todo - respect node->quote here, too.  Which also means updating the parser.
+                */
                FR_SBUFF_RETURN(fr_value_box_print, out, &node->data, e_rules);
                goto done;
 
@@ -1161,8 +1164,7 @@ static ssize_t xlat_print_node(fr_sbuff_t *out, xlat_exp_t const *head, fr_sbuff
 
                } else {
                        FR_SBUFF_IN_STRCPY_LITERAL_RETURN(out, "(");
-                       xlat_print_node(out, node->child, e_rules);
-                       FR_SBUFF_IN_CHAR_RETURN(out, ' ');
+                       xlat_print_node(out, node->child, e_rules); /* prints a space after the first argument */
 
                        /*
                         *      @todo - when things like "+" support more than 2 arguments, print them all out

diff --git a/src/tests/unit/xlat/expr.txt b/src/tests/unit/xlat/expr.txt
index fbf12bd94ed80ee06ac3b3a5e45a938de23c42cf..c03ccdc2bc95851650da47606c05a4c5fd47b9ee 100644 (file)
--- a/src/tests/unit/xlat/expr.txt
+++ b/src/tests/unit/xlat/expr.txt
@@ -45,10 +45,10 @@ match (1 + (%{Service-Type} == Framed-User))
 #  Strings of various forms
 #
 xlat_expr &Filter-Id == "foo"
-match (%{Filter-Id} == \"foo\")
+match (%{Filter-Id} == "foo")
 
 xlat_expr "foo" == "bar"
-match (\"foo\" == \"bar\")
+match ("foo" == "bar")
 
 # note '/' is a prefix, not "divide by 24".
 # and a useless cast is removed
@@ -82,11 +82,11 @@ xlat_expr &Filter-Id
 match %{Filter-Id}
 
 xlat_expr %{md5:foo} + "foo"
-match  (%{md5:foo} + \"foo\")
+match  (%{md5:foo} + "foo")
 
 #  We can name the xlat's, tho we don't need to
 xlat_expr %(op_add:4 3) + 6
-match ((4  + 3) + 6)
+match ((4 + 3) + 6)
 
 
 #

diff --git a/src/tests/unit/xlat/purify.txt b/src/tests/unit/xlat/purify.txt
index 0c5aa0f39fd7ae4b15dea991b7a9c2eaeb605c88..78583efc1aa27ce62cdc5429011bbed5c40a4218 100644 (file)
--- a/src/tests/unit/xlat/purify.txt
+++ b/src/tests/unit/xlat/purify.txt
@@ -1,5 +1,8 @@
 proto-dictionary radius
 
+#xlat_purify %(op_add:4 3) + 6
+#match 13
+
 #
 #  xlat_expr, but purified
 #
@@ -48,7 +51,7 @@ match (1 + (%{Service-Type} == Framed-User))
 #  Strings of various forms
 #
 xlat_purify &Filter-Id == "foo"
-match (%{Filter-Id} == \"foo\")
+match (%{Filter-Id} == "foo")
 
 xlat_purify "foo" == "bar"
 match no
@@ -85,11 +88,15 @@ xlat_purify &Filter-Id
 match %{Filter-Id}
 
 xlat_purify %{md5:foo} + "foo"
-match  (%{md5:foo} + \"foo\")
+match  (%{md5:foo} + "foo")
 
 #  We can name the xlat's, tho we don't need to
+#
+#  And naming the xlat's means that they don't set up
+#  with the magic token field, so optimizations don't work?
+#
 xlat_purify %(op_add:4 3) + 6
-match ((4  + 3) + 6)
+match ((4 + 3) + 6)
 
 
 #