Re: [PATCH 2/8] builtin: define a symbol_op for a generic op acting on integer

[Ramsay Jones <ramsay@ramsayjones.plus.com>]

On 10/04/2021 14:30, Luc Van Oostenryck wrote:
> This can be used to define some generic (polymorphic) builtin
> with a signature like:
> 	op(T)
> 	op(T, T)
> 	op(T,T, ... T)
> where T is some integer type.
> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  builtin.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  builtin.h |  2 ++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/builtin.c b/builtin.c
> index ff03dbab9a06..f03bf109c818 100644
> --- a/builtin.c
> +++ b/builtin.c
> @@ -390,6 +390,56 @@ static struct symbol_op overflow_p_op = {
>  };
>  

I must apologize in advance, I've got a head cold, I'm tired and
should probably not be commenting (especially since I am only
skimming these patches), but ...

>  
> +///
> +// Evaluate the arguments of 'generic' integer operators.
> +//
> +// All arguments must be the same basic integer type and
> +// their number comes from the prototype and is already checked.
> +static int evaluate_generic_int_op(struct expression *expr)
> +{
> +	struct symbol *fntype = expr->fn->ctype->ctype.base_type;
> +	struct symbol_list *types = NULL;
> +	struct symbol *ctype = NULL;
> +	struct expression *arg;
> +	struct symbol *t;
> +	int n = 0;
> +
> +	PREPARE_PTR_LIST(fntype->arguments, t);

So, t is used to iterate over the 'fntype->arguments' (which is to say
the generic type T), ...

> +	FOR_EACH_PTR(expr->args, arg) {
> +		struct symbol *type;
> +
> +		if (++n == 1) {
> +			t = arg->ctype;

which is then set to the first argument type, here ...

> +			if (!arg || !(type = arg->ctype))
> +				return 0;
> +			if (type->type == SYM_NODE)
> +				type = type->ctype.base_type;
> +			if (!type)
> +				return 0;
> +			if (type->ctype.base_type != &int_type || type == &bool_ctype)
> +				goto err;
> +		} else {
> +			t = ctype;

... and thereafter (for 2, 3, ...) to NULL. (ctype is not set to anything
and it is initialized to NULL above).

> +		}
> +		add_ptr_list(&types, t);

... so this 'types' list is just {arg->ctype, NULL, NULL, ...}

> +		NEXT_PTR_LIST(t);
> +	} END_FOR_EACH_PTR(arg);
> +	FINISH_PTR_LIST(t);
> +	return evaluate_arguments(types, expr->args);
> +
> +err:
> +	sparse_error(arg->pos, "non-integer type for argument %d:", n);

here, n will only ever be 1, right? (only way to get here, via goto, from
the n == 1 conditional, above).

> +	info(arg->pos, "        %s", show_typename(arg->ctype));
> +	expr->ctype = &bad_ctype;
> +	return 0;
> +}

I stopped reading here. (I probably should have stopped sooner! ;).

ATB,
Ramsay Jones

> +
> +struct symbol_op generic_int_op = {
> +	.args = args_prototype,
> +	.evaluate = evaluate_generic_int_op,
> +};
> +
> +
>  static int eval_atomic_common(struct expression *expr)
>  {
>  	struct symbol *fntype = expr->fn->ctype->ctype.base_type;
> diff --git a/builtin.h b/builtin.h
> index 9cb6728444fe..5fe77c926244 100644
> --- a/builtin.h
> +++ b/builtin.h
> @@ -14,4 +14,6 @@ struct builtin_fn {
>  
>  void declare_builtins(int stream, const struct builtin_fn tbl[]);
>  
> +extern struct symbol_op generic_int_op;
> +
>  #endif
>