xuzifu666 commented on code in PR #5200:
URL: https://github.com/apache/calcite/pull/5200#discussion_r3821371707
##########
linq4j/src/main/java/org/apache/calcite/linq4j/tree/BlockBuilder.java:
##########
@@ -603,15 +605,64 @@ private static class InlineVariableVisitor extends
SubstituteVariableVisitor {
/** Use counter. */
private static class UseCounter extends VisitorImpl<Void> {
- private final IdentityHashMap<ParameterExpression, Slot> map = new
IdentityHashMap<>();
+ /** Map each parameter to information about how it is used. */
+ private final IdentityHashMap<ParameterExpression, ParameterUse> map = new
IdentityHashMap<>();
Review Comment:
If I understand correctly, similar issues may still arise in scenarios like
the following:
```
final int x = 1 / i;
while (c) {
use(x);
}
```
In the original semantics, the assignment `x = 1 / i` executes before the
`while` condition is evaluated; if `i == 0`, an exception should be thrown even
if `c` is `false`.
However, the current `UseCounter` does not override `visit(WhileStatement)`,
`visit(ForStatement)`, or `visit(ForEachStatement)`; consequently, the
inherited `VisitorImpl` counts the loop body as a standard, unconditional
access. As a result, if `x` is used only once within the loop body, it might
still be inlined into the loop body:
```
while (c) {
use(1 / i);
}
```
When c == false, the exception is optimized away again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
