remove bounce through tmpl code for %{...}

Instead, we create an XLAT_GROUP to hold the child xlat.

We then also create a "hoist" flag, which only exists for a
group node, and isn't in the flags.

update the debug printer to match.

Update the evaluation code to look for the "hoist' flag,
and then hoist the result instead of creating a value-box group.

Note that the result may be empty.  In which case nothing is
added to the output dcursor.  For xlat function arguments, this
highlights the need to have each argument expanded into its own
group.

diff --git a/src/lib/unlang/xlat.h b/src/lib/unlang/xlat.h
index 3086ae862104bb8cc5635c54444b886536cee2dd..baefee54094460a833d8b43872dccc9e258c5c81 100644 (file)
--- a/src/lib/unlang/xlat.h
+++ b/src/lib/unlang/xlat.h
@@ -110,13 +110,13 @@ typedef struct xlat_s xlat_t;
  *
  */
 typedef struct {
-       bool                    needs_resolving;//!< Needs pass2 resolution.
-       bool                    pure;           //!< has no external side effects, true for BOX, LITERAL, and some functions
-       bool                    impure_func;    //!< xlat contains an impure function
-       bool                    can_purify;     //!< if the xlat has a pure function with pure arguments.
+       uint8_t                 needs_resolving : 1;    //!< Needs pass2 resolution.
+       uint8_t                 pure : 1;               //!< has no external side effects, true for BOX, LITERAL, and some functions
+       uint8_t                 impure_func : 1;        //!< xlat contains an impure function
+       uint8_t                 can_purify : 1;         //!< if the xlat has a pure function with pure arguments.
 
-       bool                    constant;       //!< xlat is just tmpl_attr_tail_data, or XLAT_BOX
-       bool                    xlat;           //!< it's an xlat wrapper
+       uint8_t                 constant : 1;           //!< xlat is just tmpl_attr_tail_data, or XLAT_BOX
+       uint8_t                 xlat : 1;               //!< it's an xlat wrapper
 } xlat_flags_t;
 
 extern fr_table_num_sorted_t const xlat_action_table[];

diff --git a/src/lib/unlang/xlat_eval.c b/src/lib/unlang/xlat_eval.c
index c9c156b5b2d598230a96d3ef0cff2173e33f68bd..8dddb1bfc1c9e4cc9b328317274a0ffd0d2f1570 100644 (file)
--- a/src/lib/unlang/xlat_eval.c
+++ b/src/lib/unlang/xlat_eval.c
@@ -1097,6 +1097,19 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_dcursor_t *out,
                XLAT_DEBUG("** [%i] %s(child) - continuing %%{%s ...}", unlang_interpret_stack_depth(request), __FUNCTION__,
                           node->fmt);
 
+               /*
+                *      Hoist %{...} to its results.
+                *
+                *      There may be zero or more results.
+                */
+               if (node->hoist) {
+                       while ((arg = fr_value_box_list_pop_head(result)) != NULL) {
+                               talloc_steal(ctx, arg);
+                               fr_dcursor_insert(out, arg);
+                       }
+                       break;
+               }
+
                MEM(arg = fr_value_box_alloc(ctx, FR_TYPE_GROUP, NULL));
 
                if (!fr_value_box_list_empty(result)) {
@@ -1385,10 +1398,12 @@ static int xlat_sync_stringify(TALLOC_CTX *ctx, request_t *request, xlat_exp_hea
                char *escaped;
 
                /*
-                *      Groups only come about because of quoted strings.
+                *      Groups commonly are because of quoted strings.
+                *
+                *      However, we sometimes have a group because of %{...}, in which case the result is just
+                *      a leaf value.
                 */
-               if (node->type == XLAT_GROUP) {
-                       fr_assert(vb->type == FR_TYPE_GROUP);
+               if ((node->type == XLAT_GROUP) && (vb->type == FR_TYPE_GROUP)) {
                        fr_assert(node->quote != T_BARE_WORD);
 
                        if (xlat_sync_stringify(vb, request, node->group, &vb->vb_group, escape, escape_ctx) < 0) return -1;

diff --git a/src/lib/unlang/xlat_priv.h b/src/lib/unlang/xlat_priv.h
index 75d7dd2ded53785448ca050cf15baa60ff2e21c1..8419ed2060115f02be745701e40da7b9631a066c 100644 (file)
--- a/src/lib/unlang/xlat_priv.h
+++ b/src/lib/unlang/xlat_priv.h
@@ -161,7 +161,10 @@ struct xlat_exp_s {
 #endif
 
        union {
-               xlat_exp_head_t *group;         //!< children of a group
+               struct {
+                       xlat_exp_head_t *group;         //!< children of a group
+                       uint8_t         hoist : 1;      //!< it's a group, but we need to hoist the results
+               };
 
                /** An tmpl_t reference
                 *

diff --git a/src/lib/unlang/xlat_tokenize.c b/src/lib/unlang/xlat_tokenize.c
index ead27df4bb502299e4c2a3aeddab7a97822b74b3..0b7fdab4fc717b809208ff014885e50a02718c61 100644 (file)
--- a/src/lib/unlang/xlat_tokenize.c
+++ b/src/lib/unlang/xlat_tokenize.c
@@ -601,17 +601,14 @@ static CC_HINT(nonnull(1,2)) int xlat_tokenize_expansion(xlat_exp_head_t *head,
        /*
         *      It must be an expression.
         *
-        *      We wrap the xlat in a tmpl, so that the result is just a value, and not wrapped in another
-        *      XLAT_GROUP, which turns into a wrapper of FR_TYPE_GROUP in the value-box.
+        *      We wrap the xlat in a group, and then mark the group to be hoisted.
         */
        {
-               xlat_exp_head_t *child;
                tmpl_rules_t my_rules;
 
                fr_sbuff_set(in, &m_s);         /* backtrack to the start of the expression */
 
-               MEM(node = xlat_exp_alloc(head, XLAT_TMPL, NULL, 0));
-               MEM(node->vpt = tmpl_alloc(node, TMPL_TYPE_XLAT, T_BARE_WORD, "", 1));
+               MEM(node = xlat_exp_alloc(head, XLAT_GROUP, NULL, 0));
 
                if (t_rules) {
                        my_rules = *t_rules;
@@ -620,7 +617,7 @@ static CC_HINT(nonnull(1,2)) int xlat_tokenize_expansion(xlat_exp_head_t *head,
                        t_rules = &my_rules;
                }
 
-               ret = xlat_tokenize_expression(node->vpt, &child, in, &attr_p_rules, t_rules);
+               ret = xlat_tokenize_expression(node, &node->group, in, &attr_p_rules, t_rules);
                if (ret <= 0) {
                        talloc_free(node);
                        return ret;
@@ -632,14 +629,15 @@ static CC_HINT(nonnull(1,2)) int xlat_tokenize_expansion(xlat_exp_head_t *head,
                }
 
                xlat_exp_set_name(node, fr_sbuff_current(&m_s), fr_sbuff_behind(&m_s));
-               tmpl_set_name_shallow(node->vpt, T_BARE_WORD, node->fmt, fr_sbuff_behind(&m_s));
+               node->flags = node->group->flags;
 
-               tmpl_set_xlat(node->vpt, child);
-               xlat_exp_insert_tail(head, node);
+               /*
+                *      Print it as %{...}.  Then when we're evaluating a string, hoist the results.
+                */
+               node->flags.xlat = true;
+               node->hoist = true;
 
-               child->flags.xlat = true;
-               node->flags = child->flags;
-               fr_assert(tmpl_xlat(node->vpt) != NULL);
+               xlat_exp_insert_tail(head, node);
 
                (void) fr_sbuff_next(in); /* skip '}' */
                return ret;
@@ -1038,6 +1036,8 @@ ssize_t xlat_print_node(fr_sbuff_t *out, xlat_exp_head_t const *head, xlat_exp_t
 
        if (!node) return 0;
 
+       if (node->flags.xlat) FR_SBUFF_IN_CHAR_RETURN(out, '%', '{');
+
        switch (node->type) {
        case XLAT_GROUP:
                if (node->quote != T_BARE_WORD) FR_SBUFF_IN_CHAR_RETURN(out, fr_token_quote[node->quote]);
@@ -1046,7 +1046,12 @@ ssize_t xlat_print_node(fr_sbuff_t *out, xlat_exp_head_t const *head, xlat_exp_t
 
                if (xlat_exp_next(head, node)) {
                        if (c) FR_SBUFF_IN_CHAR_RETURN(out, c);
-                       FR_SBUFF_IN_CHAR_RETURN(out, ' ');      /* Add ' ' between args */
+
+                       /*
+                        *      This thing is %{...}, and we don't print a space between the last argument
+                        *      and the '}'.
+                        */
+                       if (!node->flags.xlat) FR_SBUFF_IN_CHAR_RETURN(out, ' ');      /* Add ' ' between args */
                }
                goto done;
 
@@ -1106,9 +1111,7 @@ ssize_t xlat_print_node(fr_sbuff_t *out, xlat_exp_head_t const *head, xlat_exp_t
 
                if (tmpl_contains_xlat(node->vpt)) { /* xlat and exec */
                        if (node->vpt->quote == T_BARE_WORD) {
-                               if (node->flags.xlat) FR_SBUFF_IN_CHAR_RETURN(out, '%', '{');
                                xlat_print(out, tmpl_xlat(node->vpt), NULL);
-                               if (node->flags.xlat) FR_SBUFF_IN_CHAR_RETURN(out, '}');
                        } else {
                                FR_SBUFF_IN_CHAR_RETURN(out, fr_token_quote[node->vpt->quote]);
                                xlat_print(out, tmpl_xlat(node->vpt), fr_value_escape_by_quote[node->quote]);
@@ -1215,6 +1218,8 @@ ssize_t xlat_print_node(fr_sbuff_t *out, xlat_exp_head_t const *head, xlat_exp_t
        FR_SBUFF_IN_CHAR_RETURN(out, close);
 
 done:
+       if (node->flags.xlat) FR_SBUFF_IN_CHAR_RETURN(out, '}');
+
        return fr_sbuff_used_total(out) - at_in;
 }
 
@@ -1931,6 +1936,8 @@ int xlat_resolve(xlat_exp_head_t *head, xlat_res_rules_t const *xr_rules)
                         */
                        if (tmpl_resolve(node->vpt, xr_rules->tr_rules) < 0) return -1;
 
+                       if (xlat_tmpl_normalize(node) < 0) return -1;
+
                        node->flags.needs_resolving = false;
                        node->flags.pure = tmpl_is_data(node->vpt);
                        break;

diff --git a/src/tests/keywords/xlat-alternation b/src/tests/keywords/xlat-alternation
index 719476823e622d53513f5e1a74068ccf181bc1a6..b6ea1005d4ac9172ad4745755c296fcbe1324d19 100644 (file)
--- a/src/tests/keywords/xlat-alternation
+++ b/src/tests/keywords/xlat-alternation
@@ -10,7 +10,7 @@ NAS-Identifier := "bar"
 #  First choice
 #
 result_string := "%{Filter-Id || NAS-Identifier}"
-if (!(result_string == 'foo')) {
+if (result_string != 'foo') {
        test_fail
 }
 
@@ -19,7 +19,7 @@ if (!(result_string == 'foo')) {
 #
 request -= Filter-Id[*]
 result_string := "%{Filter-Id || NAS-Identifier}"
-if (!(result_string == 'bar')) {
+if (result_string != 'bar') {
        test_fail
 }
 
@@ -27,7 +27,7 @@ if (!(result_string == 'bar')) {
 #  Multiple things in an alternation
 #
 result_string := %{%{Filter-Id} || "%{NAS-Identifier} foo"}
-if (!(result_string == 'bar foo')) {
+if (result_string != 'bar foo') {
        test_fail
 }
 
@@ -35,7 +35,7 @@ if (!(result_string == 'bar foo')) {
 #  Alternation is empty
 #
 result_string := "%{Filter-Id || ''}"
-if (!(result_string == '')) {
+if (result_string != '') {
        test_fail
 }
 
@@ -48,7 +48,7 @@ request -= NAS-Identifier[*]
 #  Both sides are failing, so the assignment returns a NULL string
 #
 result_string := "%{Filter-Id || NAS-Identifier}"
-if (!(result_string == "")) {
+if (result_string !='') {
        test_fail
 }
 

diff --git a/src/tests/unit/xlat/purify.txt b/src/tests/unit/xlat/purify.txt
index c9f53cdb0e6536a379f64257c6be50bacf2b2d52..7693a4e8bdebe711df579fafff904d6b775e017d 100644 (file)
--- a/src/tests/unit/xlat/purify.txt
+++ b/src/tests/unit/xlat/purify.txt
@@ -227,8 +227,11 @@ match "hello 3"
 xlat_purify "hello " + (string)%{1 + 2} + " bob"
 match "hello 3 bob"
 
+#
+#  @todo - xlat_purify doesn't hoist results.
+#
 xlat_purify "hello %{1 + 2} bob"
-match "hello %{3} bob"
+match "hello 3  bob"
 
 #
 #  New syntax!