Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 316697

Summary: [debug] Stepping through callin bindings in a role file shows bogus locations in the team
Product: [Tools] Objectteams Reporter: Stephan Herrmann <stephan.herrmann>
Component: OTDTAssignee: Stephan Herrmann <stephan.herrmann>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3    
Version: 1.4   
Target Milestone: 0.8 M7   
Hardware: PC   
OS: Linux   
Whiteboard: trac
Attachments:
Description Flags
fixes and test adjustments
none
more fixes & tests
none
yet more fixes and tests none

Description Stephan Herrmann CLA 2010-06-13 09:07:47 EDT
(originally from http://trac.objectteams.org/ot/ticket/226)

Using the same modified version of flight bonus as in 
http://trac.objectteams.org/ot/ticket/225 (Subscriber in a role file), 
steps like the callin binding and lifting are displayed in the debugger using 
line numbers from the role but the team file, not the role file. 

Normal code (role methods, even the guard predicate) are correctly shown in the 
role file, though. 

The difference is due to the fact that callin binding and lifting are actually generated into the team.

---
today's comments:

according to http://trac.objectteams.org/ot/ticket/248#comment:6
some bridge methods will be created with source positions of the team class
declaration. Need to check whether this is tolerable.
Comment 1 Stephan Herrmann CLA 2010-06-13 09:13:44 EDT
Similar problems from http://trac.objectteams.org/ot/ticket/235:

When stepping through the OTSample-Flightbonus, it is not possible to watch the 
code of roles like GUIConnector$__OT__FlightBonus$__OT__Collector. Note that this role is nested in a role file and additionally contains copy-inherited code. 

In some cases the debugger says that the source could not be found, in other 
cases the correct line number is displayed in the wrong file: GUIConnector. 

How to reproduce: 
* Try to step into FlightBonus$Item.earnCredits which is implicitly inherited but 
  also intercepted from the GUIConnector 
* Or override any of the implicitly inherited methods in Collector (just c
Comment 2 Stephan Herrmann CLA 2011-04-21 20:28:55 EDT
Created attachment 193896 [details]
fixes and test adjustments

This patch resolves several issues in this field:
- nested teams did not get any SMAP generated 
  (TypeDeclaration.createSmapGenerator explicitly excluded role&team)
- fixed hierarchy of SMAP generators: 
  - all basic handling in AbstractSmapGenerator
  - make Role.. and TeamSmapGenerators independent subclasses
    (may in the future require an explicit NestedTeamSmapGenerator)
  - activate code in TeamSmapGenerator that was commented out
- improved computation of file names considering nested teams
  (and role files) and stripping off "__OT__".
- don't merge file infos if only the simple name equals
- avoid adding a file layer to the stratum just for the
  special lines STEP_OVER and STEP_INTO

Cleanup: renamed LineInfoReminder to LineInfoCollector.

Also adjusted the stratum tests (no longer expect unnecessary
entries for phantom role).

Not yet: within recordCredits() the copy-inherited callouts getStart()
and getDestination() cannot be shown correctly.
Comment 3 Stephan Herrmann CLA 2011-04-24 18:06:30 EDT
Created attachment 193973 [details]
more fixes & tests

More fixes:

Correctly handle line numbers of synthetic methods during copy inheritance.
When tsuper method is read from .class file this already worked as desired, 
but CopyInheritance.copySyntheticMethod(..) failed to set & remap line numbers.
Fixed by this chain:
+ SyntheticMethodBinding stores lineNumber, computed in relevant constructors.
+ explicitly transfer line number during copy inheritance using the current
  LineNumberProvider (thus mapping orig line to a new line)
+ ClassFile consumes pre-computed line number if present

Improve source position for synthetic statements in lifting constructors.
Previously those statements always had the position of the role class header,
which could lead to discontinuous source positions, thus confusing the
SMAP generator. Fixed like this:
+ If the lifting constructor exists in source code (synthetic statements
  being merged into that ctor) use the position of the ctor header.

Cleanup and fix lookup of the file name for each layer in the stratum.
+ Always consider nested teams, i.e., be ready to find the appropriate
  enclosing type from a deeply nested role.
+ Never continue travelling out when we see a role file.
+ Avoid duplicate and similar code, e.g., don't provide AST and binding based
  variants of the same code.
+ Most of this lookup is now pulled up to AbstractSmapGenerator.

Tests have been cleaned up:
+ pull up common stuff to AbstractSourceMapGeneratorTest
+ fix a few classpath issues and correctly pass compiler options
and a new test based on OTSample-FlightBonus has been added.
Comment 4 Stephan Herrmann CLA 2011-04-25 18:19:01 EDT
Created attachment 194025 [details]
yet more fixes and tests

Expanded the test to capture the initially reported issue
(FlightBonus.Subscriber is now a role file).

Include synthetic positions in re-mapping for role files:
+ Asks MethodModel whether a special AstGenerator should be used:
  + ask LineNumberProvider for remapping the original source line
  + let CompilationResult allocate a source position for the synthetic line
  + ProblemHandler.record(..) protects against using synthetic positions
    during error reporting (would cause AIOOBE during access to source)
+ Currently applies to these methods when generated from a RoFi:
  + callin wrapper
  + liftTo method
This replaces an old fakery: Previously, methods generated into a team from
source in a RoFi of this team were created with the RoFi's CompilationResult
and during codeGen the lineEnds from the CompilationResult were temporarily 
used by the codeStream. Both fakes are no longer needed.
TODO: check if embedded real code fragments (e.g., param-mapping-exprs)
need to be re-positioned, too.

Fixed mapping of lines (LineNumberProvider):
+ don't reuse a mapping with repeatCount n for another region 
  starting at the same line but with repeatCount m > n.

Make TeamSmapGenerator ready for real work.

Fixed a bug in MessageSend:
+ include our synthetic arguments in the region associated to the previous pc

Make source-pos setting more complete (Lifting and AstGenerator)
Comment 5 Stephan Herrmann CLA 2011-04-26 13:11:50 EDT
The various improvements have been committed between r1445 and r1453.

Seems to work to satisfaction - currently no further action planned.
Comment 6 Stephan Herrmann CLA 2011-04-29 20:01:18 EDT
Verified for 0.8M7 using build 201104281905