Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dataflow: unexpected receiver type in MethodAccessNode in a subclass #5174

Open
artempyanykh opened this issue Jun 24, 2022 · 6 comments
Open
Labels

Comments

@artempyanykh
Copy link
Contributor

artempyanykh commented Jun 24, 2022

Snippet:

class Super1<T> {
  T methodInner() {
    return null;
  }
}

class Sub1<U> extends Super1<U> {
  U method() {
    return methodInner();
    //      ^------------------- (this).methodInner
  }
}

When superclass's method is invoked in a subclass, the receiver of the method points to superclass' type rather than subclass':

actual:

receiver.type = Super1<T>

expected:

receiver.type = Sub1<U>

This is problematic because I can't use receiver's type to determine if I need to substitute type params in method's type for the subclass.


The same is true for FieldAccessNode.

@artempyanykh
Copy link
Contributor Author

Actually, changing the method access above to use an explicit this changes the receiver type to Sub1<U>.

this.methodInner();

Since semantically it's the same thing, I believe the type of the receiver should reflect the actual type of the accessed object.

@mernst mernst linked a pull request Jun 29, 2022 that will close this issue
@mernst
Copy link
Member

mernst commented Jun 29, 2022

Does the new method MethodAccessNode.getOwner(), defined in #5184, solve your problem?

(It's not the right thing to change the receiver's type, but I agree that it is useful to know the class that defines the method!)

@artempyanykh
Copy link
Contributor Author

Thanks for your response @mernst!

#5184 doesn't quite solve the problem. I can already get this info from MethodAccessNode n like this:

ElementUtils.enclosingTypeElement(n.getMethod())

Which gives the class that defined the method.

The problem is that when this is implicit in a method access, n.getReceiver().getType() returns the same thing as getOwner (modulo TypeElement vs TypeMirror) rather than the actual type of the object that the method is accessed on.

I'm not sure whether this is by design, but my expectation was that the type of the receiver should reflect the actual type of the object: in the example above I'd expect the type of an implicit this receiver to be the actual subclass' type Sub1<U> and not Super1<T>.

Does this make sense?

@mernst
Copy link
Member

mernst commented Jul 11, 2022

@artempyanykh The current state of branch issue5174 in https://github.com/mernst/checker-framework fixes the issue for your test case. It isn't ready to merge because it does not correctly handle method calls to outer class methods without an explicit receiver. But, you could experiment with it.

@mernst
Copy link
Member

mernst commented Jul 25, 2022

@artempyanykh Did you have a chance to experiment with branch issue5174 in https://github.com/mernst/checker-framework?

@artempyanykh
Copy link
Contributor Author

Hi @mernst. Sorry, I was sick and slow to respond.

I played with issue5174. As you said, it fixed issues like the one in the description. But in some cases it still seems to pick the wrong type, e.g. anon inner classes:


interface I { void call(); }

public abstract class Test {        
    int outerMethod(Integer outer) {
        return outer;
    }
    
    void test1() {
        new I(){
            public void call() {
                int r1 = outerMethod(42);
                //         (this).outerMethod
                //            ^--- should be Test; actual is <anonymous I>
            }
        };
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants