Bug 378396 - Problem with AJ Editor with final + compiler problem due to the @DeclareParents(value="xxx || xxxx")
Summary: Problem with AJ Editor with final + compiler problem due to the @DeclareParen...
Status: NEW
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: aspectj inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-03 13:46 EDT by Jorelia CLA
Modified: 2012-05-08 08:53 EDT (History)
1 user (show)

See Also:


Attachments
Here is the workbench - Please read the readme.txt file. (120.94 KB, application/zip)
2012-05-03 13:47 EDT, Jorelia CLA
no flags Details
Some screen shot (393.09 KB, image/png)
2012-05-03 13:49 EDT, Jorelia CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jorelia CLA 2012-05-03 13:46:35 EDT
Build Identifier:  AJDT version: 2.1.3.e37x-20110628-1900 for Eclipse 3.7 /   AspectJ Compiler version: 1.6.12.M1

There is several problems probably related to the fact that the @DeclareParents(value="xxx || xxxx") is not supported with the version 1.6.12.M1.

Here the symptoms:
 1) When a class has final keyword, the class that is weaving with the aspect is not "tagged" inside the IDE.

 2) If we remove the final keyword, the aspect is displayed under the IDE

 3) Despit of that, the aspect is not executed. We think that is because the involved classes are not under the scope @DeclareParents as we cannot use the syntax @DeclareParents(value="xxx || xxxx").



Reproducible: Always
Comment 1 Jorelia CLA 2012-05-03 13:47:42 EDT
Created attachment 215019 [details]
Here is the workbench - Please read the readme.txt file.
Comment 2 Jorelia CLA 2012-05-03 13:49:51 EDT
Created attachment 215020 [details]
Some screen shot
Comment 3 Jorelia CLA 2012-05-03 13:53:45 EDT
Edit the class, and add final or remove it.

 - \NestedWeb2\src\com\nested\aspects\LoggingBehavior.java
 - \NestedWeb2\src\com\nested\web\validator\impl
 or 
 - \NestedWeb2\src\com\nested\web\converter\NameConverter.java

I've also put a System.out into the NameConverter class.

at C:\Temp\NestedWeb.log, it should produce the following trace, but no trace about the converter logging.

2012-05-03 13:28:36,903 DEBUG [com.nested.aspects.LoggingBehavior$ServantLogger] -  - DesktopController.doItemChangeListener(..) - Entering... - params: Param1 : 'org.richfaces.event.ItemChangeEvent[source=org.richfaces.component.UITabPanel@1daf28a]'
2012-05-03 13:28:36,904 DEBUG [com.nested.aspects.LoggingBehavior$ServantLogger] -  - DesktopController.doItemChangeListener(..) - Object dump - com.nested.web.controller.DesktopController@18209bf
2012-05-03 13:28:42,209 DEBUG [com.nested.aspects.LoggingBehavior$ServantLogger] -  - PanelOneController.doNewList() - Entering...
2012-05-03 13:28:42,210 DEBUG [com.nested.aspects.LoggingBehavior$ServantLogger] -  - PanelOneController.doNewList() - Object dump - com.nested.web.controller.PanelOneController@4de186
2012-05-03 13:28:42,214 DEBUG [com.nested.aspects.LoggingBehavior$ServantLogger] -  - PanelOneController.validate() - VALIDATOR - return: true
2012-05-03 13:28:42,279 DEBUG [com.nested.aspects.LoggingBehavior$ServantLogger] -  - ListItemController.initialize(..) - Entering initializer... - Param1 : 'TAB_ITEM-01-1'
Comment 4 Andrew Clement CLA 2012-05-06 12:13:04 EDT
I just wanted to clarify a few things as there is a lot going on here.

> There is several problems probably related to the fact that the
> @DeclareParents(value="xxx || xxxx") is not supported with the version
> 1.6.12.M1.

If you are using the || notation you can't rely on the build results (because right now it produces the exception on compilation).  So I presume the issues you are seeing with final are just related to a regular DeclareParents and not using the || version?  If I look in the project LoggingBehaviour aspect in the attached project there is no || so I presume I can ignore that from this bug report, it is a separate issue to the 'final' problem.

How does this project behave on an AJDT that includes the up to date AspectJ 1.7 dev builds? The same issue with final?

Can you email me the 'lib' folder with all the jars in (andrew.clement AT gmail.com) - there are too many for me to download them individually from the internet.  (A pom for the project would be ideal).
Comment 5 Jorelia CLA 2012-05-07 08:38:47 EDT
Ok, let's clarify the issue - I'm using 1.6.12M1 - We cannot upgrade to 1.7 because of RAD which use 3.6 of Eclipse.

For the issue, effectively we do not use @DeclareParents( xxx || xxx), but if you pay attention to the screen shots and the code source of loggingBehavior, you can see some pointcut related to converterLogger.

Then, the issue is related to adding "final" or not "final" to class "NameConverter" and the behavior of compiler as well as of AJ Editor. The AJ editor told us that the pointcut/advice is good but in the fact, the advice is not executed. Adding the keyword or remove it, has a different behavior from the compiler. Why ?

The test that you can do, is to launch the project under your environment and to test under 1.7 AJDT if you observe the same issue. If no, then you can close the issue.

I've made a link to the @DeclareParents( xxx || xxx) because, it seems there is a link because my initial willing was to have :
@DeclareParents(NameConverter || Validator || Controller).


As we can have only one class, I've put only Controller class but AJ Editor and Compiler behave as it was the NameConverter and Validator class due to the pointcut. But in reality, the advice is not performed even if the AJ editor told us it was good.

Regards,
Comment 6 Andrew Clement CLA 2012-05-07 11:43:30 EDT
Hi,

thanks for the clarification.  Really I need the project to compile so I can investigate properly - any chance of that lib folder?  I've downloaded some of the jars referenced in the readme but I don't seem to be able to cover all the dependencies from the source.  I don't need to run it really, I just need to compile it so that I can look at the woven bytecode.

final does affect pointcut matching at times, as it can make certain situations impossible (for example I would know a final type cannot be subclassed by something else that adds an additional interface).

Without the dependencies it is hard for me to recreate/investigate.  if you wanted to try something I'd say what does it do if you use a code style declare parents (don't add the additional targets, just try it as code style), since code style declare parents is implemented in a very different way to annotation style.  Annotation style has far less test coverage than code style, because it is newer, and as I say it is kind of deprecated in favour of @DeclareMixin.
Comment 7 Andrew Clement CLA 2012-05-07 12:23:43 EDT
ok, I have this building now.  Not sure if I have exactly the same jars as you, but it is compiling.  Proceeding with investigation...
Comment 8 Andrew Clement CLA 2012-05-07 13:35:47 EDT
As I mentioned above, final can influence matching and that is what is happening here.

Also the advice markers don't always indicate a match, if they have a small question mark suffixed that means the advice will run *if* a test passes at runtime (basically we couldn't determine it is a definitive match at compile time and are having to check something at runtime).  In the case of this pointcut:

converterLogger() && this(log)

the runtime test that is inserted into the code looks a bit like this:

if (this instanceof Loggable) {
  calladvice();
}

(because we don't *definitively* know if 'this' at the time that method runs will be an instanceof Loggable)

Because your declareparents doesn't make NameConverter into a Loggable, that test will always fail and the advice won't run.

Why advise that method at all?  Well NameConverter could be subclassed at some point:

class SubNameConverter extends NameConverter implements Loggable;

Now that obviously isn't happening but the fact that it could means AspectJ considers the method in NameConverter a potential match and leaves in the guard to check for the instance implementing Loggable.

Now if you make that NameConverter final, AspectJ realises you can't subclass it, so you can't introduce Loggable via a subclass.  This means at compile time it can be confident this method will never match, so it doesn't insert a runtime test and doesn't leave a marker in the editor.

If you modify your DeclareParents to target NameConverter, you will see that the around advice icon appears, and this time without a '?' indicating we statically know this is a match because NameConverter implements Loggable.


Seems like the best thing for me to do is put an AspectJ 1.7 into the eclipse 3.6 stream so you can use the DeclareParents and target all the types you want to.  We are working on doing this now.
Comment 9 Jorelia CLA 2012-05-07 16:02:35 EDT
Thank you very much for the explanation - Yes, it would be nice to upgrade but it not under my control since we use RAD 8.0.4(under eclipse 3.6) from IBM. We need to wait an update for RAD.

http://www-01.ibm.com/software/awdtools/developer/application/ ->Feature and benefits....
Comment 10 Andrew Clement CLA 2012-05-07 16:36:01 EDT
by upgrade I just meant that if I push an AspectJ 1.7 into AJDT for Eclipse 3.6 then it will produce a new dev build for Eclipse 3.6 that you could install into RAD from here:

http://download.eclipse.org/tools/ajdt/36/dev/update

I'll put an AspectJ in now, so in a couple of hours there will be a dev build suitable for Eclipse 3.6 that you can use.
Comment 11 Jorelia CLA 2012-05-08 08:53:23 EDT
Great - Thank you very much, I saw also that version 1.7 is out. Cool. FYI, I'm presently in communication with Weld debug team. You can follow the conversation : https://issues.jboss.org/browse/WELD-1123 - Best regards.