On Wed, 2010-06-09 at 00:24 +1000, Max Matveev wrote:
> >> If I change primary() to not pick arguments of NOP type I get the
> >> "correct" result.
>
> kenj> I don't think this is the correct fix ... see my earlier patch.
> I didn't see how the first change to use tspan from arg1 was fixing
> the problem of rippling the wrong tspan up the expression tree by selection
> the "wrong" branch to ripple up.
The problem was not tspan "rippling" up the tree (as for many nodes
tspan is never inspected), the problem was that the iterator in
cndMatch_inst() was looping over the arg1 node, but using values from
the x node to guard and set the iteration limit
This needs to be running over the set of instances that need filtering
for (n = 0; n < arg1->tspan; n++) {
It used to be running over something else (which happened to be the
length of the uncompiled regex)
for (n = 0; n < x->tspan; n++) {
So as soon as the number of instances to be filtered was smaller than
the arbitrary value of x->tspan then i gets too big and
regexec((regex_t *)arg2->ring, m->inames[i], ...
can only lead to badness.
> kenj> The "node" in the expression tree is a Pascal variant record dosed with
> kenj> steroids and EPO ... tspan is a synonym for string length in the case
> of
> kenj> a constant string (sem == SEM_CHAR).
> I find this very confusing - in most other places tspan seems to be
> used to count number of instances and therefor truth bits in the
> instance array, why use it to encode string length in the first place?
a) because there is no other place to encode the length of the string,
and b) because the same data structure is used for operands (values)
where the array of concept needs tspan (and remember these can span
instances and/or hosts and/or time samples), operators (no array at all)
and strings (length approx equal to size of char[] a la C in a perverse
sort of way)
And yes it is very confusing!
> kenj> What was matginally wrong until my second round of fixes was
> kenj> that nvals and nsmpls were strangely not 1 for string
> kenj> constants, and then the value of a regex quitely got turned
> kenj> from a string into a compiled regex with nothing in the
> kenj> expression node to suggest this had happened.
> I think the change of setting tspan to 1 when converting string to
> regex is the key to fixing the problem.
No ... it just makes things cleaner ... I believe using the correct node
in the expression tree for the iterator range is the key.
|