361441 - Use "new" evaluateToX methods rather than evaluate

Status: NEW

(Core :: XSLT, defect)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

We should use the evaluateToX methods more, both implement them and call them. We have to watch out for bad call chains though, so calling evaluateToBoolean on an expression should not call evaluateToBoolean->evaluate->evaluateToNumber.

Comment 1

[Peter Van der Beken [:peterv]]

Have a patch, but it'll have to wait for attachment 247792 [details] [diff] [review] to land.

Comment 2

[Peter Van der Beken [:peterv]]

Attachment: v1

This adds quite a bit of code :-/. We could do away with some of it if we stop overriding evaluate() when we have a more specific evaluateToX(). That would replace one virtual function call (evaluate) with three: evaluate, getReturnType and evaluateToX.

Comment 3

[Peter Van der Beken [:peterv]]

Attachment: v1 (diff -w)

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I'm not too exited about using getReturnType. Why not create a set of base classes, txBaseNumberExpr, txBaseBoolExpr etc. txBaseNumberExpr would lack evaluateToNumber but would let all other functions forward to evaluateToNumber.

You could simplify txCoreFunctionCall/txEXSLTFunctionCall two ways:

Either put a switch(mType) statement in each evaluteToX. In evaluateToBoolean you'd directly handle the functions that return a bool. The functions that return a number forward directly to evaluateToNumber etc. Same thing in the other evaluate functions.

The other solution is to put a switch(descriptTable[mType].mReturnType) in the beginning of evaluateToBoolean and forward directly to other evaluate functions if needed, and then a switch(mType) to handle the functions that do return a boolean.


What do you think?

Comment 5

[Peter Van der Beken [:peterv]]

Attachment: v2

As I suspected, that actually adds even more to the codesize.

Comment 6

[Peter Van der Beken [:peterv]]

Attachment: v2.1

This gives a much smaller codesize increase.

Comment 7

[Peter Van der Beken [:peterv]]

Attachment: v2.1 (diff -w)

Comment 8

[Peter Van der Beken [:peterv]]

Attachment: v2.1

Updated to trunk.

Comment 9

[Peter Van der Beken [:peterv]]

Attachment: v2.1