On Thu, 12 Jan 2023 19:12:27 GMT, Archie L. Cobbs <d...@openjdk.org> wrote:
>> Uhm. Turns out I probably did not understand the filter correctly, and now
>> I'm more dubious about what it actually does. Say you have this hierarchy:
>>
>>
>> interface A { }
>> class B {
>> B() {
>> A a = (A)this;
>> ...
>> }
>> }
>> class C extends B implements A { }
>> ```
>>
>> Pathological case, I know. But the filtering will end up dropping the
>> expression Ref on the floor, right? (because B and A are unrelated).
>
>> But the filtering will end up dropping the expression Ref on the floor,
>> right? (because B and A are unrelated).
>
> Ah, I see what you mean.
>
> Here's a more complete example:
>
> public class CastLeak {
>
> public CastLeak() {
> ((CastLeak)(Runnable)this).mightLeak();
> }
>
> public void mightLeak() {
> }
> }
>
> That would be a leak for any subclass that implements `Runnable`. Yet no
> warning is generated.
>
> So the filtering by expression type is going to potentially create false
> negatives. But it also eliminates a bunch of false positives. And the false
> negatives are probably all somewhat pathological like the example above.
>
> So I still think it's worth doing.
Ok - I thought false negative was the thing to absolutely avoid - and that was
the no. 1 concern. I think a possible approach to keep both the filtering and
the code more or less similar to what you have, is to save the type of the
expression in the ExprRef. Then, I believe that, at the end of scan() you can
just get whatever type is there, and check that it's the correct one, if not
drop it.
-------------
PR: https://git.openjdk.org/jdk/pull/11874
