Bug #2556
closedIncorrect copying of subexpressions during FLWOR optimization
100%
Description
This arose from investigation of bug #2547, though it is not directly related. Discovered from code inspection, there are no known symptoms, but the code is clearly incorrect.
In FLWORExpression.optimize(), there are particular conditions under which the variable declared in a "let" clause is inlined (that is, references to the declared variable V are replaced by the expression E to which it is bound). In all cases the expression E is first copied.
-
But there are many situations where this copy is unnecessary, for example in the simple case of let $V := E return $V, which can be replaced by E without first copying E.
-
And there are some situations where the copy is insufficient, for example in the case of let $V := $A return $V+$V+$V, where the resulting expression $A+$A+$A should contain three different references to the same variable rather than three copies of the same variable. (Not much is likely to go wrong if there are three copies of the same variable reference in a simple case like this. But the expression tree is no longer a well formed tree, and subsequent processes walking the tree could trip up on this.)
Updated by Michael Kay almost 9 years ago
- Status changed from New to In Progress
As a first step, I have changed FLWORExpression.optimize() so it converts single-clause for/let expressions to a ForExpression or LetExpression as the first thing it does, which saves going through a lot of convoluted logic designed to handle more complex and less common cases.
Updated by Michael Kay almost 9 years ago
- Status changed from In Progress to Resolved
- Fix Committed on Branch 9.7 added
As a further change, I have added logic to ExpressionTool.replaceSelectedSubexpressions() to include logic about when to copy the replacement subexpression before inserting it into the tree.
Updated by O'Neil Delpratt almost 9 years ago
- % Done changed from 0 to 100
- Fixed in Maintenance Release 9.7.0.2 added
Bug fix applied in the Saxon 9.7.0.2 maintenance release
Updated by O'Neil Delpratt almost 9 years ago
- Status changed from Resolved to Closed
Please register to edit this issue