On Tue, 16 Apr 2024 19:15:14 GMT, Archie Cobbs <[email protected]> wrote:
>> Please review several fixes and improvements to the `this-escape` lint >> warning analyzer. >> >> The goal here is to apply some relatively simple logical fixes that improve >> the precision and accuracy of the analyzer, and capture the remaining >> low-hanging fruit so we can consider the analyzer relatively complete with >> respect to what's feasible with its current design. >> >> Although the changes are small from a logical point of view, they generate a >> fairly large patch due to impact of refactoring (sorry!). Most of the patch >> derives from the first two changes listed below. >> >> The changes are summarized here: >> >> #### 1. Generalize how we categorize references >> >> The `Ref` class hierarchy models the various ways in which, at any point >> during the execution of a constructor or some other method/constructor that >> it invokes, there can be live references to the original object under >> construction lying around. We then look for places where one of these >> `Ref`'s might be passed to a subclass method. In other words, the analyzer >> keeps track of these references and watches what happens to them as the code >> executes so it can catch them trying to "escape". >> >> Previously the `Ref` categories were: >> * `ThisRef` - The current instance of the (non-static) method or constructor >> being analyzed >> * `OuterRef` - The current outer instance of the (non-static) method or >> constructor being analyzed >> * `VarRef` - A local variable or method parameter currently in scope >> * `ExprRef` - An object reference sitting on top of the Java execution stack >> * `YieldRef` - The current switch expression's yield value(s) >> * `ReturnRef` - The current method's return value(s) >> >> For each of those types, we further classified the "indirection" of the >> reference, i.e., whether the reference was direct (from the thing itself) or >> indirect (from something the thing referenced). >> >> The problem with that hierarchy is that we could only track outer instance >> references that happened to be associated with the current instance. So we >> might know that `this` had an outer instance reference, but if we said `var >> x = this` we wouldn't know that `x` had an outer instance reference. >> >> In other words, we should be treating "via an outer instance" as just >> another flavor of indirection along with "direct" and "indirect". >> >> As a result, with this patch the `OuterRef` class goes away and a new >> `Indirection` enum has been created, with values `DIRECT`, `INDIRECT`, and >> `OUTER`. >> >> #### 2. Track the types... > > Archie Cobbs has updated the pull request incrementally with one additional > commit since the last revision: > > Synchronize TreeInfo.isExplicitThisReference() with PR #18088. looks sensible to me ------------- Marked as reviewed by vromero (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/16208#pullrequestreview-2006363434
