[augeas-devel] [PATCH] Generic square lens
David Lutterkort
lutter at redhat.com
Tue Aug 7 21:16:21 UTC 2012
On Mon, 2012-08-06 at 21:42 +0200, Francis Giraldeau wrote:
> Lens square now takes lenses instead of regular expressions as arguments, as follow:
>
> square left body right
This is very cool. If it does indeed let us put all those quote problems
to rest, it's a huge step forward.
I get these errors when I try to build with this patch:
lens.c:403:14: error: no previous prototype for 'lns_copy_prim' [-Werror=missing-prototypes]
lens.c: In function 'lns_make_square':
lens.c:426:19: error: unused variable 'val' [-Werror=unused-variable]
I suspect you don't have --enable-compile-warnings=error turned on; have
a look at the autogen target in Makefile.maint for how you should set up
your environment. The attached patch fixes the trivial issues; just fold
into yours.
> diff --git a/src/get.c b/src/get.c
> index c1e206c..b8e5195 100644
> --- a/src/get.c
> +++ b/src/get.c
> @@ -50,7 +50,8 @@ struct state {
> struct seq *seqs;
> char *key;
> char *value; /* GET_STORE leaves a value here */
> - char *square; /* last L_DEL from L_SQUARE */
> + char *square_left;
> + char *square_right;
Do we really need to store that in the state ? Wouldn't it be possible
to figure out square_left and square_right in get_square by looking at
the match for the first and last child of the square lens ?
> static struct tree *get_lens(struct lens *lens, struct state *state) {
> struct tree *tree = NULL;
>
> switch(lens->tag) {
> case L_DEL:
> tree = get_del(lens, state);
> + record_square(lens, state);
If we can figure out square_left and square_right in get_square, we
wouldn't have to do this.
> @@ -398,35 +400,51 @@ struct value *lns_make_maybe(struct info *info, struct lens *l, int check) {
> return make_lens_value(lens);
> }
>
> +struct lens *lns_copy_prim(struct lens *lens, struct info *info) {
> + struct lens *ret = NULL;
> + struct value *val = NULL;
> +
> + val = lns_make_prim(lens->tag, ref(info), ref(lens->regexp),
> + ref(lens->string));
> + ERR_NOMEM(val == NULL, info);
> + ret = ref(val->lens);
> + unref(val, value);
> + error:
> + return ret;
> +}
> +
> /* Build a square lens as
> * key REG . lns . del REG MATCHED
> * where MATCHED is whatever the key lens matched (the inability to express
> * this with other lenses makes the square primitve necessary
> */
> -struct value *lns_make_square(struct info *info,
> - struct regexp *reg,
> - struct lens *lns, int check) {
> - struct value *key = NULL, *del = NULL;
> +struct value *lns_make_square(struct info *info, struct lens *l1,
> + struct lens *l2, struct lens *l3, int check) {
> +
> struct value *cnt1 = NULL, *cnt2 = NULL, *res = NULL;
> - struct lens *sqr = NULL;
> + struct lens *sqr = NULL, *l1_copy = NULL, *l3_copy = NULL;
> + struct value *val = NULL;
>
> - res = lns_make_prim(L_KEY, ref(info), ref(reg), NULL);
> - if (EXN(res))
> - goto error;
> - key = res;
> + /* supported types: L_KEY . body . L_DEL or L_DEL . body . L_DEL */
> + if (l3->tag != L_DEL || (l1->tag != L_DEL && l1->tag != L_KEY))
> + return make_exn_value(info, "Supported types: (key lns del) or (del lns del)");
>
> - res = lns_make_prim(L_DEL, ref(info), ref(reg), NULL);
> - if (EXN(res))
> + /* deepcopy l1 and l3 */
> + l1_copy = lns_copy_prim(l1, info);
> + ERR_NOMEM(l1_copy == NULL, info);
> + l3_copy = lns_copy_prim(l3, info);
> + ERR_NOMEM(l3_copy == NULL, info);
> +
> + res = typecheck_square(info, l1, l3);
> + if (res != NULL)
> goto error;
> - del = res;
>
> - // typechecking is handled when concatenating lenses
> - res = lns_make_concat(ref(info), ref(key->lens), ref(lns), check);
> + /* do not increase ref of copies */
> + res = lns_make_concat(ref(info), l1_copy, ref(l2), check);
> if (EXN(res))
> goto error;
> cnt1 = res;
> -
> - res = lns_make_concat(ref(info), ref(cnt1->lens), ref(del->lens), check);
> + res = lns_make_concat(ref(info), ref(cnt1->lens), l3_copy, check);
> if (EXN(res))
> goto error;
> cnt2 = res;
> @@ -439,22 +457,30 @@ struct value *lns_make_square(struct info *info,
> sqr->recursive = cnt2->lens->recursive;
> sqr->rec_internal = cnt2->lens->rec_internal;
> sqr->consumes_value = cnt2->lens->consumes_value;
> + /* flag first and last child of concat to be under square lens
> + * Used to check if the string matched by these lenses are equals */
> + lens_child_first(sqr->child)->in_square = 1;
> + lens_child_last(sqr->child)->in_square = 1;
If we compute left and right during get_square, we also do not need to
annotate the first and last child with in_square .. even if that doesn't
work, couldn't we push that flag into the state in get.c and turn it on
in get_square ? That would avoid copying the lens.
> +/* FIXME: avoid code duplication */
What's the duplication ?
> +/* helper to access first and last child */
> +#define lens_child_first(l) l->children[0]
> +#define lens_child_last(l) l->children[l->nchildren - 1]
There's a bunch of parens missing to make these safe, e.g.
#define lens_child_last(l) ((l)->children[(l)->nchildren - 1])
> diff --git a/tests/modules/pass_square.aug b/tests/modules/pass_square.aug
> index f41761d..fff56e9 100644
> --- a/tests/modules/pass_square.aug
> +++ b/tests/modules/pass_square.aug
I haven't read them too closely, but do
> @@ -8,17 +8,24 @@ let dels (s:string) = del s s
> *************************************************************************)
>
> (* Simplest square lens *)
> -let s = store /[yz]/
> -let sqr0 = [ square "x" s ] *
> -test sqr0 get "xyxxyxxyx" = { "x" = "y" }{ "x" = "y" }{ "x" = "y" }
> -test sqr0 put "xyx" after set "/x[3]" "z" = "xyxxzx"
> +let s = store /[ab]/
> +let sqr0 =
> + let k = key "x" in
> + let d = dels "x" in
> + [ square k s d ] *
> +test sqr0 get "xaxxbxxax" = { "x" = "a" }{ "x" = "b" }{ "x" = "a" }
> +test sqr0 put "xax" after set "/x[3]" "b" = "xaxxbx"
Minor nit: the set should either be set "/x[2]" or set "/x[last() + 1]",
x[3] is confusing here.
David
-------------- next part --------------
A non-text attachment was scrubbed...
Name: square.patch
Type: text/x-patch
Size: 830 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/augeas-devel/attachments/20120807/72a96ac6/attachment.bin>
More information about the augeas-devel
mailing list