Comment 1 Andrew Clement 2016-04-22 11:58:27 EDT

Hi, I'm sure there is an issue here but it isn't obvious and must be down to some slight subtlety. Here is my pass at a test:

==== Play.java ===
package Parcelable;

import org.aspectj.lang.annotation.*;
import java.lang.annotation.*;

@Retention(RetentionPolicy.RUNTIME)
@interface Constructor {}

aspect X {
  declare @constructor: !@Parcelable.Constructor (InterParcelableImpl+ && !InterParcelableImpl).new(): @Parcelable.Constructor;
}

class InterParcelableImpl {public InterParcelableImpl() {}}
class Mid extends InterParcelableImpl {}
class One extends InterParcelableImpl { public One() {}}
class Two extends InterParcelableImpl { @Parcelable.Constructor public Two() {}}
class Three extends Mid {}
====

ajc -1.8 -showWeaveInfo Play.java
' void Parcelable.Three.new()' (Play.java) is annotated with @Parcelable.Constructor constructor annotation from 'Parcelable.X' (Play.java:10)

'public void Parcelable.One.new()' (Play.java:15) is annotated with @Parcelable.Constructor constructor annotation from 'Parcelable.X' (Play.java:10)

' void Parcelable.Mid.new()' (Play.java) is annotated with @Parcelable.Constructor constructor annotation from 'Parcelable.X' (Play.java:10)

And then using javap -verbose I look at One/Two/Three and Mid. There are no duplicate annotations anywhere.

So we need to work out how what you are doing is different to this:
- are you binary weaving into these types (were they compiled earlier) or are you compiling everything from source?
- are the type declarations different to what I've done, i.e. mine are just 1/2 deep, are yours much deeper than that?
- does my annotation declaration basically match yours, are you using meta annotations like @Inherited or anything? Are you using runtime retention?

Comment 2 Eduard Matsukov 2016-04-23 07:55:37 EDT

I had annotation in this way declared:

@Target(TYPE)
@Retention(RUNTIME)
public @interface Parcelable {

    @Target(CONSTRUCTOR)
    @Retention(RUNTIME)
    public @interface Constructor {}

    @Target(FIELD)
    @Retention(RUNTIME)
    public @interface Field {}
}

I'll try to make separate annotations and write the result.

But why I wrote a bug. I had the similar instruction:
declare @field: !@Parcelable.Field !static * (InterParcelableImpl+ && !InterParcelableImpl).* : @Parcelable.Field;

And it works fine as I expect: declares only once per field.

Except the annotations declaration everything else in your code is similar to mine, so maybe this bug is in annotations declaration.

Comment 3 Eduard Matsukov 2016-04-23 08:47:03 EDT

I've tried to make @Constructor declaration separate as standalone annotation. But no luck. I see this in .class file:


@Constructor
@Constructor
public Contact() {
    ajc$xpoint_parcel_engine_ParcelableProcessor$Injector$localAspectOf().ajc$afterReturning$xpoint_parcel_engine_ParcelableProcessor$Injector$1$944aaca4(this, ajc$tjp_0);
}

Comment 4 Andrew Clement 2016-04-25 13:35:55 EDT

Certainly declaring an inner annotation could be a clue, I have no AspectJ testcases that use inner annotations. I changed my progam:

===
package foo;

import org.aspectj.lang.annotation.*;
import java.lang.annotation.*;

@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
@interface Parcelable {

  @Target(ElementType.CONSTRUCTOR)
  @Retention(RetentionPolicy.RUNTIME)
  @interface Constructor {}

  @Target(ElementType.FIELD)
  @Retention(RetentionPolicy.RUNTIME)
  @interface Field {}
}

aspect X {
  declare @constructor: !@Parcelable.Constructor (InterParcelableImpl+ && !InterParcelableImpl).new(): @Parcelable.Constructor;
}

class InterParcelableImpl {public InterParcelableImpl() {}}
class Mid extends InterParcelableImpl {}
class One extends InterParcelableImpl { public One() {}}
class Two extends InterParcelableImpl { @Parcelable.Constructor public Two() {}}
class Three extends Mid {}
===

But that works fine. It could be the order in which things get compiled but I can't guess what that failing order might be, ideally I need a failing sample, then I can probably fix it pretty quickly.

Comment 5 Eduard Matsukov 2016-04-27 05:25:31 EDT

Created attachment 261292 [details]
aspectj parcelable engine

Hi, Andrew!

Thanks for your help a lot! Here is all related files. Hope they will help to locate the problem.

Comment 6 Andrew Clement 2016-04-28 15:02:46 EDT

The easiest thing to help me is something I can download, compile and it shows the issue. I tried grabbing that code but I don't have the android jars around to compile it against, or even know if a simple ajc is enough to build it if I did just have an android jar around. Is there a maven/gradle build file for that code?  I doubt just looking at source will fix it, I really need to see failing code, then I can address it pretty quick I imagine.

Comment 7 Eduard Matsukov 2016-05-11 11:40:42 EDT

Sorry for a delay. I found how this issue may be reproduced. Could you try just to add an extending class parents/interface list via aspectj before injecting annotations over constructor. If that won't reproduce, I'll try to write a sample proj, which shows my behaviour. I tried different combinations and got duplication only when my class is being extended or implemented some interfaces from aspectj.

Thank's a lot for your responses! :)

Comment 8 Andrew Clement 2016-05-11 16:50:55 EDT

I've been tinkering with the test program, but not getting very far. I've tried enhancing the test program in comment 2 to add a declare parents implements with a marker interface but that didn't help. Then I tried an extends with an empty class, that didn't help. Then I tried an intertype declaration of a new constructor, but that didn't help. (intertype declarations are handled differently, I wondered if the ITDd constructors would somehow be misbehaving). 

You mentioned:

> Could you try just to add an extending class parents/interface list via aspectj before injecting annotations over constructor. 

By 'before' do you mean in a separate compile step? Or just put the declare parents ahead of the declare @constructor?  The ordering doesn't really matter because constructs that modify the type structure are always done first (so wherever you specify declare parents with respect to declare @constructor, the declare parents will be done first). I haven't tried separate compilation in two steps.