paulk-asert commented on PR #3002: URL: https://github.com/apache/groovy/pull/3002#issuecomment-6599067181
@blackdrag re "why is it legal to remove it?": for a *reference* to a local variable or parameter (as opposed to its declaration, which the `vexp != localVariable` guard leaves alone), the node's `INFERRED_TYPE` is only ever written by this method. Each visit computes it from the declaration's type plus the current flow (temporary) type info, and nothing earlier in the same visit puts a type on that node. An assignment target gets its type stored after the target has been visited, and the SC transformers and classgen write only after STC has finished. So when we get here, any `INFERRED_TYPE` already on the node was left by an earlier visit of the same node in a different flow context: a revisit, or a transform sharing the node across branches. The fresh computation is the authoritative one. Before this change the code was asymmetric. It overwrote the type when there was narrowing, but left the old value in place when there wasn't, and that stale value is the leak. "No narrowing here" is exactly what an absent `INFERRED_TYPE` means to readers: they fall back to the variable's declared or inferred type. So removing it makes the node say what the current flow context says. @daniellansun: 1. Flow typing is keyed on the variable, not the node: `getInferredTypeFromTempInfo` looks up the accessed variable. So distinct reference nodes for the same variable still narrow independently, and the clear only touches the node being visited. 2. GROOVY-12428 has landed. The PR merges cleanly and the STC/SC/typing/bugs suites pass on top of it. A shared node across switch cases (`case Map:` then `default:`) fails on master with a `GroovyCastException` because the case's Map narrowing reaches `default`, and passes with this change. I'll add that as a third test. The negated form (`!(p instanceof Map) ? … : …`), where the narrowing is in the branch visited last, works too. 3. Agreed. Only `INFERRED_TYPE` is flow-derived here. `DECLARATION_INFERRED_TYPE` and other keys aren't touched. -- 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]
