Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 316148 - [otjld] [compiler] Merging of class-based and binding-based precedences is underspecified
Summary: [otjld] [compiler] Merging of class-based and binding-based precedences is un...
Status: VERIFIED FIXED
Alias: None
Product: Objectteams
Classification: Tools
Component: OTJ (show other bugs)
Version: 0.7   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 0.7 M4   Edit
Assignee: Stephan Herrmann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-08 10:52 EDT by Stephan Herrmann CLA
Modified: 2010-06-11 18:08 EDT (History)
1 user (show)

See Also:


Attachments
OTJLD clarification (1.89 KB, patch)
2010-06-08 14:12 EDT, Stephan Herrmann CLA
no flags Details | Diff
implementation (3.70 KB, patch)
2010-06-08 14:14 EDT, Stephan Herrmann CLA
no flags Details | Diff
Additional fix (9.56 KB, patch)
2010-06-09 07:18 EDT, Stephan Herrmann CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stephan Herrmann CLA 2010-06-08 10:52:08 EDT
OTJLD ยง4.8(d) currently reads:
" Multiple precedence statements

  All precedence statements are collected at the outer-most team. At that level
  all precedence declarations involving the same base method are merged using
  the C3 algorithm [3]. It is an error to declare incompatible precedence lists 
  that cannot be merged by the C3 algorithm."

While this ensures that the actual precedence will _conform_ to all individual
precedence declarations, this fails to determine the actual ordering
when class-based and binding-based precedence declarations need merging.

Consider this team:

public team class MyTeam {
  precedence MyRoleB.bl1, MyRoleA;
    protected class MyRoleB playedBy MyBase {
      precedence bl1, bl2;
      void rm() {
        System.out.println("RoleB.bl1");
      }
                
      bl1: rm <- before bm;
                
      void rm2() {
        System.out.println("RoleB.bl2");
      }
      bl2: rm2 <- before bm;
    }

    protected class MyRoleA playedBy MyBase {
      void rm() {
        System.out.println("RoleA");
      }
      rm <- before bm;
    }
}

calling bm() currently produces:

RoleB.bl1
RoleA
RoleB.bl2

whereas the following would be legal too:

RoleB.bl1
RoleB.bl2
RoleA

Actually, the latter seems more intuitive because it gives priority
to inner precedence declarations ("bl1, bl2") over outer precedence
declarations ("MyRoleB.bl1, MyRoleA").

The OTJLD should be updated to disambiguate the behavior and the 
implementation be adjusted accordingly.
Comment 1 Stephan Herrmann CLA 2010-06-08 14:12:04 EDT
Created attachment 171439 [details]
OTJLD clarification

Addition to the OTJLD that clarifies how declarations are merged.
Comment 2 Stephan Herrmann CLA 2010-06-08 14:14:28 EDT
Created attachment 171440 [details]
implementation

This patch contains the implementation part of the fix.

Corresponding tests are test4127_precedenceDeclaration15() f.
Comment 3 Stephan Herrmann CLA 2010-06-08 14:26:58 EDT
Fix has been committed as r438.
Comment 4 Stephan Herrmann CLA 2010-06-09 07:18:18 EDT
Created attachment 171511 [details]
Additional fix

The patch from comment 2 caused a regression in 
MethodMappingBindingTest#testPrecedence1() f.

Investigation revealed that we have always performed too much
transformation on the AST: collecting precedences in the enclosing team
should not modify the AST but work on bindings only. Otherwise the
DOM AST is affected, too, as witnessed by this regression.

Also one error message is improved in this patch: when precedences are 
in conflict say which ones (either symmetrically or point to one list 
and mention the other in the message).
Comment 5 Stephan Herrmann CLA 2010-06-11 18:08:35 EDT
Verified for M4 using build 201006111053.