Bug 310501 - intertype declared inner classes
Summary: intertype declared inner classes
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: DEVELOPMENT   Edit
Hardware: PC Windows 7
: P3 enhancement (vote)
Target Milestone: 1.6.11   Edit
Assignee: aspectj inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 317086
Blocks: 316554
  Show dependency tree
 
Reported: 2010-04-26 13:14 EDT by Andrew Clement CLA
Modified: 2010-12-09 16:47 EST (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Clement CLA 2010-04-26 13:14:28 EDT
Use cases have recently come up for the ITD'ing of inner types, something like this:

class C {
  class Foo.Inner {
  }
}

class Foo {
  // gets an inner class of Inner
}

syntax will need more thought, may require the intertype {} syntax enhancement.
Comment 1 Andrew Clement CLA 2010-05-07 16:25:09 EDT
this is tricky.  Brainstorming around what needs doing:

- grammar changes to support 'class Foo.Inner {'.  must allow for parameterization of the target type too.

- ASTNode for the new entity IntertypeMemberClassDeclaration

- how is the inner type represented on the disk?  Without doing any work it will look like an inner type of the aspect (this may not be a problem)

- Resolution: need to get the intertype declared class to behave as if declared in the target.  This means both sides: (1) intercepting resolution of everything inside the inner type to conside its parent to be the target type rather than the aspect. (2) putting it into the targets list of resolved member type declarations so that local references in the target can be resolved.

- Execution: need to determine what code to generate that will satisfy the results of resolution.  Using the traditional way in which ITDs are supported, references made in the target will be forwarded to the real inner type (that exists in the aspect).  However, a copy of the real class may need generating with the right name under the target type to allow reflection to behave as expected.   references from the inner type to its host will need appropriate rewriting to use accessors for fields/methods - depending upon visibility.  For non-static inner class ITDs we will need to look after a this$ type field that maintains the instance of the inner type per instances of the outer type.

- The joinpoint model must not be damaged.  So think about advising interactions with a real inner class.  The pulling of that inner class out into an aspect must neither damage joinpoints that exist for referring to the inner class NOR joinpoints that exist for the inner class referring to its original 'host'.  This can be implemented through appropriate use of the EffectiveSignatureAttribute support.

Looks like a months worth of work right now...
Comment 2 Andrew Clement CLA 2010-05-07 20:34:53 EDT
need to think about binary weaving, incremental compilation, relationships and AJDT impact too.
Comment 3 Andrew Clement CLA 2010-05-10 17:55:27 EDT
The Roo case is behaving for me.  Basically we mirror what is done for inter type declared fields/methods.  Alongside the 'finder' for these methods (which lives in source types) we have a finder for declared extra member types.  It is managed in exactly the same way and is consulted when attempting to resolve references in the normal way (by overriding getMemberType in SourceTypeBinding).

I think next I want to add a relationship for it to the model and check what AJDT does with the construct.   With appropriate warnings about this being a feature under development, I don't see why people can't start trying it out.
Comment 4 Andrew Clement CLA 2010-06-11 19:27:44 EDT
a week of hell.  Some very low points with the increase in complexity in the compilation algorithms making it seem like it wasn't worth doing, but finally I have something that is reasonable.  The first cut is committed into AJDT and into AspectJ.  The one thing I would like to finish off with is deciding on the permissions - what are the supported permissions, shall I just require things to be public (otherwise spit out a compiler limitation message)
Comment 5 Andrew Clement CLA 2010-11-25 14:55:52 EST
the first variant of this functionality did not support separate compilation, it only worked for one full batch compilation of all the pieces (the aspect making the type ITD and the target of that ITD).  This was because we were slipping the inner type onto the target to enable resolution of references to it, but on disk it still had the old name (based on the originating aspect).  Thus when the target was referred to through the classpath for a latter compile, it didn't look like it had an inner.

So really when applying the type ITD we have to make it part of the target.  Giving it the appropriate name and ensuring the outer type get the right innerclass attribute setup.

Having implemented that, we have separate compilation working.  Now incremental compilation is broken...

If the type being targeted with the ITD is changed and an incremental compile attempted, we have 'lost' the inner type (it was attached to the target during the first build).  Although we pull the aspect back in for compilation (as a binary type binding) and it has an innerclass attribute set, there is a check in the JDT ClassFileReader which does not allow inner types to have a name other than one relating to the outer type.  To fault in the inner type binding as a member of the aspect we have to prevent that name check.

With these changes, incremental compilation now appears to work.  I'll put it into AJDT and we can try it out.
Comment 6 Andrew Clement CLA 2010-12-09 16:47:20 EST
Working much better in 1.6.11.  May need to explore non-static inner types at some point.