Bug 469759

Comment 1 Stephan Herrmann 2016-02-14 17:45:27 EST

Brainstorming a solution:

We need to distinguish between
(1) super-base method is unbound: it can be directly called using invokespecial
(2) super-base method is bound: it's code will have been moved to _OT$callOrig

For (1) we can devise a strategy using special method ids:

- construct a special boundMethodId for the super-called base method,
  e.g. build boundMethodId so that normal Ids are even numbers and Ids
  for super access are Id+1. 
- let the base-super call add 1 to the boundMethodId
- inside _OT$callOrig dispatch to the super method using invokespecial
  - when filling _OT$callOrig we check if a base-super call affects a given
    base method
    - if so, add a case with Id+1, instead of inlined code this issues an
      invokespecial to the super classes "real" base method/
    - ensure that this Id+1 is not used in sub base classes
      (need another tweak to handle multiple super-base calls at different
       levels of the same base method hierarchy).


For (2) we should be fine by generating an invokespecial when the team calls _OT$callOrig. Unfortunately, this invocation is in the static Team._OT$terminalCallNext() (why is it static? can't we override that if needed?)

Or, if using the same modified boundMethodId, inside _OT$callOrig() an odd id directly leads to invoking super._OT$callOrig(..id-1..), but only do this in the class that actually handles id. Hm, still not enough to cope with all forms of dynamic binding for _OT$callOrig and overriding base methods.


Realistically speaking, (2) should be uncommon. So I will probably start by working on (1) and raise exceptions if any of it's assumptions is violated.

Comment 2 Stephan Herrmann 2016-02-16 09:36:17 EST

Documenting the implementation strategy:

Updated the OTDynCallinBindings attribute to support passing more information from compiler to OTDRE:
Add a new flag to the "flags" field, BASE_SUPER_CALL, value is 16. This flag is set whenever a bound role method contains at least one base-super call.


Modified signatures of o.o.Team#_OT$callNext() and _OT$terminalCallNext():
In both methods pass 3-valued baseCallFlags:
- 0 = not a base call
- 1 = regular base call
- 2 = base-super call
If 2, increment the boundMethodId for the call to MyBase._OT$callOrig(..).
(I left the previous variant of _OT$callNext() for now to ease migration of my development environment).


Compiler changes to support the above:

Move argument packing for base calls into BaseCallMessageSend.prepareSuperAccess() where we have the information #isSuperAccess. From this flag create an intLiteral of value 1 or 2 for the call to _OT$callNext(). To compensate for this move, a few methods needed adjustment to understand the different structure of the base call's arguments (TransformStatementsVisitor#isRecursiveCall(), also reporting baseCallDoesntMatchRoleMethodSignature() has been moved right into prepareSuperAccess() (easiest to detect here)).

Create the OTSpecialAccessAttribute for base-super only targeting OTRE weaving. Old flag DYN_SUPER_METHOD_ACCESS is now obsolete and has been removed.

Setting the new flag BASE_SUPER_CALL had to be delayed, when creating the attribute we don't yet have that information, it is computed only during BCMS.analyseCode().


Changes in OTDRE:

Transport the new flag BASE_SUPER_CALL via this (lengthy) chain:
- CallinBindingsAttribute extracts BASE_SUPER_CALL from #flags
- store in CallinBindingsAttribute.MultiBinding#requireBaseSuperCall
- AsmClassVisitor to transfer it from MultiBinding to Binding
- store in Binding#requireBaseSuperCall
- store in WeavingTask#requireBaseSuperCall
- pass it down into ABC#moveCodeToCallOrig() & MoveCodeToCallOrigAdapter()

Set IDs issued by Member#getKey() by increments of 2 to create space for intermediate IDs for base-super calls.

Create new switch label for boundMethodId+1 only when baseSuperRequired is set in the MoveCodeToCallOrigAdapter: 
- unpack & load arguments
- invokespecial
- (box and) return the result


With these changes the following tests now pass:
- test4513_basecallSuperAccess1
- test4513_basecallSuperAccess2

OTOH, these still fail, witnessing the lack of support for (2) in comment 1:
- test4513_basecallSuperAccess4
- test4513_basecallSuperAccess5

Comment 3 Stephan Herrmann 2016-03-22 18:52:30 EDT

(In reply to Stephan Herrmann from comment #2)
> OTOH, these still fail, witnessing the lack of support for (2) in comment 1:
> - test4513_basecallSuperAccess4
> - test4513_basecallSuperAccess5

I created bug 490231 for those.

Marking as fixed (wrt the main part) for 2.5 M6.
commit 6b9a43a2b7349cb02a2fed73b3e12ff3351cf5b3

follow-ups (migrate to new signature of Team._OT$callNext()):
commit 8eb5372be26833c2f7019b394df1a35a0637c269
commit bf340a97572f633220fa274d8fe48c17bb86d653
commit 7651846f879214113abe3a571fcb0bca30383aea