Need to decide how to handle forwarding semi-stubs

[stereotype441]

There's an ugly corner case in the front end's forwarding stub handling logic that we need to figure out how to handle properly. I'm calling it a "forwarding semi-stub". Consider the following code (adapted from pkg/front_end/testcases/runtime_checks_new/abstract_override_becomes_forwarding_stub.dart):

class B {
  void f(num x) {}
}
abstract class I<T> {
  void f(T x);
}
class C extends B implements I<num> {
  void f(num x);
}

From one point of view, C::f is not a forwarding stub, because it's an actual abstract method that appears in the user's source code; it's not inserted by the front end. This is the point of view that makes sense for the analyzer, since it cares only about what appears in the user's source code and doesn't need to know about runtime type checks.

From another point of view, C::f is a forwarding stub, because it's a place where a runtime type check needs to be inserted in order to ensure that the appropriate error is thrown when code like this is executed:

main() {
  I<Object> i = new C();
  i.f("oops");
}

We need to decide how this should be represented in kernel. I will follow up with a comment with more information about how it is currently represented.

I will also add a language_2 test verifying that the correct type checks occur in this situation, as well as analyzer tests to make sure that the analyzer behaves correctly.

[stereotype441]

Currently the front end tries to represent a semi-forwarding stub as follows:

• Procedure.isForwardingStub = false
• Procedure.function.body = (call to super)
• Procedure.isAbstract = false

However, after it sets this up, KernelFunctionBuilder.body= winds up setting the body to null, so as a result the semi-forwarding stub winds up looking just like an abstract method without a body, which is no good.

[stereotype441]

Note: borrowing the terminology from #31562 (comment), a forwarding semi-stub needs to have a "super target" (since at runtime there is a super method that needs to be invoked) but it does not need an "interface target" (since the interface target is only needed to ensure that analyzer navigation and error messages refer to a method that exists in the source code, and a forwarding semi-stub corresponds to a method that actually exists in the source code).

[stereotype441]

CC: @sjindel-google

[sjindel-google]

Why can't we represent them the same way as regular forwarding stubs? I.e.:

• Procedure.isForwardingStub = true
• Procedure.function.body = (call to super)

We can add another bit to indicate to the analyzer the function existed as an abstract method in the original source. I feel strongly that we shouldn't complicate the semantics of forwarding stubs any more than necessary.

[stereotype441]

@sjindel-google sounds good to me. I'll add it to my list to update the front end accordingly.

[stereotype441]

@sjindel-google do you think you could add the additional bit to indicate to the analyzer that the function existed as an abstract method in the original source? (isForwardingSemiStub, or another name of your choosing) Or should I ask someone else for that?

[sjindel-google]

I can add the bit.

[stereotype441]

Thanks! I'll work on this as soon as I can.

[kmillikin]

I agree that we'd like to clean up all the forwarding stubs. Ideally, the analyzer would never see them at all.

I don't think it's really connected to the Dart 2 stable release.