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

[Luc Van Oostenryck <luc.vanoostenryck@gmail.com>]

On Tue, Apr 13, 2021 at 01:23:00AM +0100, Ramsay Jones wrote:
> On 12/04/2021 22:21, Luc Van Oostenryck wrote:
> > +	int n = 0;
> > +
> > +	PREPARE_PTR_LIST(fntype->arguments, t);
> > +	FOR_EACH_PTR(expr->args, arg) {
> 
> Hmm, now n is always 0 in the error message, so:
> 		n++;
> here?

Hehe, yes indeed.
 
> > +		if (!is_dynamic_type(t)) {
> > +			;
> > +		} else if (!ctype) {
> > +			// fist 'dynamic' type, chat that it is an integer
> 
> s/chat/check/

fixed.

> > +	return evaluate_arguments(types, expr->args);
> 
> Hmm, does this do the usual argument promotions, so e.g. an 'generic'
> 'short' gets promoted to 'int' in the prototype? I guess not, that
> would have to be done above, while adding to the types list, right?

Well, evaluate_arguments() is the normal function used to evaluate
the arguments of all function calls, so, yes, it does arguments
promotion but only when the type is not specified (so either the
'...' of varadic function or an argument of a variadic builtin's
which is declared as NULL in the corresponding struct builtin_fn
(non-variadic builtins can't have such NULL arguments because the
first NULL is used to determine its arity and this is then normaly
checked in the .args method before the evaluation)).

> Hmm, I would have to study evaluate_arguments(), but it may be worth
> a comment here?

Not here, because there is nothing special but sure, evaluate_arguments()
should be documented and even more so how struct builtin_fn should be
used (because it's much more complex since a lot of builtins doesn't
follow the way normal C declarations rules).

> 
> So, this certainly looks better. Thanks! ;-)

Thanks to you!
-- Luc