On Fri, Jan 15, 2016 at 11:53 PM, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:
> Here is a v19 :
> - avoid noisy changes
> - abort on double->int overflow
> - implement operators as functions
>
> There is still \setrandom, that I can remove easily with a green light.

Thanks for the new patch and replacing the operator stuff by functions.

+ <entry>uniformly-distributed random integer in <literal>[lb,ub]</></>
Nitpick: when defining an interval like that, you may want to add a
space after the comma. For example seg.sgml does that. It would be
good to be consistent even here. And actually you wrote [ub, lb] in
two places, this should have been reversed.

+ /* beware that the list is reverse in make_func */
s/reverse/reversed/?

}
+
#ifdef DEBUG
Some noise.

With this example:
\set cid debug(sqrt(-1))
I get that:
debug(script=0,command=1): double nan
An error would be more logical, no? You want to emulate with complex
numbers instead?

The basic operator functions also do not check for integer overflows.
Those three ones are just overflowing:
\set cid debug(9223372036854775807 + 1)
\set cid debug(-9223372036854775808 - 1)
\set cid debug(9223372036854775807 * 9223372036854775807)
debug(script=0,command=1): int -9223372036854775807
debug(script=0,command=2): int 9223372036854775807
debug(script=0,command=3): int 1
And this one generates a core dump:
\set cid debug(-9223372036854775808 / -1)
Floating point exception: 8 (core dumped)

A more general comment: what about splitting all the execution
functions into a separate file exprexec.c? evaluateExpr (renamed as
execExpr) is the root function, but then we have a set of static
sub-functions for each node, like execExprFunc, execExprVar,
execExprConst, etc? This way we would save a bit of tab-indentation,
this patch making the new code lines becoming larger than 80
characters because of all the switch/case stuff that gets more
complicated.
--
Michael