Kernel representation of forwarding stubs needs to point to "actual" interface target in outline

[stereotype441]

Consider the following program:

class A {
  void foo(int x) {}
}
abstract class I<T> {
  void foo(T x);
}
class B extends A<int> implements I {}
void bar(B b) {
  b.foo(1);
}
main() {
  bar(new B());
}

This compiles to the following kernel representation:

library;
import self as self;
import "dart:core" as core;

class A extends core::Object {
  default constructor •() → void
    : super core::Object::•()
    ;
  method foo(core::int x) → void {}
}
abstract class I<T extends core::Object> extends core::Object {
  default constructor •() → void
    : super core::Object::•()
    ;
  abstract method foo(generic-covariant-impl generic-covariant-interface self::I::T x) → void;
}
class B extends self::A implements self::I<dynamic> {
  default constructor •() → void
    : super self::A::•()
    ;
  forwarding-stub method foo(generic-covariant-impl dynamic x) → void
    return super.{self::A::foo}(x);
}
static method bar(self::B b) → void {
  b.{self::B::foo}(1);
}
static method main() → dynamic {
  self::bar(new self::B::•());
}

Note that the call from bar to b.foo has an interface target of self::B::foo, which is a forwarding stub.

When the analyzer builds its resolution information based on the kernel representation, it can't consider the call to foo to resolve to self::B::foo, since there is no such function in the user's source code.
It needs to resolve it to self::A::foo, the method that would have been the interface target if the forwarding stub hadn't been there. It can't get this information from the body of B::foo for two reasons:

(1) In a separate compilation scenario, the body of B::foo may not be present when analyzing bar; we may only have an outline for B::foo.

(2) In certain circumstances a forwarding stub is created that lacks a body, e.g.:

abstract class A {
  void foo(int x);
}
abstract class I {
  void foo(Object x);
}
class B extends A implements I {}

which has the kernel representation:

library;
import self as self;
import "dart:core" as core;

abstract class A extends core::Object {
  default constructor •() → void
    : super core::Object::•()
    ;
  abstract method foo(core::int x) → void;
}
abstract class I extends core::Object {
  default constructor •() → void
    : super core::Object::•()
    ;
  abstract method foo(core::Object x) → void;
}
class B extends self::A implements self::I {
  default constructor •() → void
    : super self::A::•()
    ;
  abstract forwarding-stub method foo(core::Object x) → void;
}

So we need the kernel representation of a forwarding stub to contain a pointer to the interface target that would have been used if the forwarding stub had not been there; and we need this information to be present in the kernel outline. The analyzer can use this information to find the "actual" (non-forwarding) interface target.

[stereotype441]

@scheglov FYI

[scheglov]

This is now high priority for Analyzer integration. We handle enough nodes that this is now the first issue we stumble upon when analyze a big codebase.

[stereotype441]

Here's an idea as to how we might unblock Konstantin while waiting for this kernel feature to be implemented: we could add the necessary information to the front end's ForwardingNode class. This would be enough that so that in a non-modular compilation scenario the analyzer would be able to get the information it needs, and that would probably be sufficient to get most tests to pass. Once the necessary kernel bits were available we could remove the hack from ForwardingNode and change the analyzer to use the kernel bits.

@scheglov @kmillikin What do you think?

[scheglov]

SGTM!

[stereotype441]

Clarification: there are two things one might mean by the "actual" target of a forwarding stub:

1. The interface target that would have been used had the forwarding stub not been present. I call this the "interface target" of the forwarding stub. Error messages based on static type analysis should use this target. The analyzer should use this target for "go to definition".
2. The method that the forwarding stub forwards to at runtime. I call this the "super target" of the forwarding stub, since it is always in the superclass chain. The VM, dart2js, and DDC should use this target to determine which method the forwarding stub should invoke at runtime, and which types should be used for type checking.

Consider:

class A {
  void f(int x) { ... }
}
abstract class I {
  void f(covariant Object x);
}
class B extends A implements I {}

B will contain a forwarding stub for f. Its interface target is I::f. Its super target is A::f.

In this bug I believe the target we're talking about is the interface target.

[stereotype441]

CL containing the proposed workaround is here: https://dart-review.googlesource.com/c/sdk/+/27460

Note: this is slightly different than what I suggested previously. Instead of storing the necessary information in the front end's ForwardingNode class, we store it in a new class called ForwardingStub (derived from Procedure). So what you should be able to do in the analyzer is:

if (interfaceTarget is ForwardingStub) {
  interfaceTarget = ForwardingStub.getInterfaceTarget(interfaceTarget);
}

(There is no need to do this in a loop; the member returned by ForwardingStub.getInterfaceTarget is guaranteed to not be a forwarding stub).

[scheglov]

Thank you!

[scheglov]

Current status: Paul's workaround with using ForwardingStub.getInterfaceTarget works well for non-SDK code. But for SDK we have no shadow AST, and still need the "actual" (non-forwarding) interface target. As you can imaging, any application call a lot of SDK code, so we eventually fail to resolve while trying to analyze.

This is blocking Analyzer integration.

[stereotype441]

Added Samir as assignee, since he touched the code necessary to fix this bug recently - see https://dart-review.googlesource.com/c/sdk/+/31921/5/pkg/kernel/binary.md#358

[sjindel-google]

I've added the field for this parameter; however, I think someone more familiar with the current hack should remove the ForwardingStub hack and populate the new field.

[scheglov]

https://dart-review.googlesource.com/c/sdk/+/34261