Bug 98952 - @AJ if pointcut
Summary: @AJ if pointcut
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: DEVELOPMENT   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 1.5.0 M3   Edit
Assignee: Adrian Colyer CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-06-08 10:28 EDT by Alexandre Vasseur CLA
Modified: 2005-09-27 09:28 EDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alexandre Vasseur CLA 2005-06-08 10:28:41 EDT
copy of my proposal for @AJ if pointcut I came up with today and that everyone
agree with
---

Here are my thought on the if() pointcut for @AJ stuff.
I don't think it is reasonable to have string based if(....) bodies so
here is the proposal I am starting on - the main idea is : the
@Pointcut annotated method acts as the if body (hence the if body is
already at hand etc etc)

so simple !
Alex

// if() is somehow implicit ie we can know we have an if when we have
// a static boolean @Pointcut annotatated method
// TODO TBD -may help toString(), parsing and tools if we require the
if() marker
@Aspect
public class IfAspect {

   static int I;

   @Pointcut("execution(* Foo.do()) && if()") // if(IfAspect.test())
   public static boolean test() {
       return I > 0;
   }

   @Pointcut("execution(* Foo.do()) && if()") //
if(IfAspect.test2(thisJoinPoint)
   public static boolean test2(JoinPoint jp) {// will require JP flags
       return jp.getSignature().getName().startsWith("baz") && I > 0;
   }

   @Pointcut("execution(* Foo.do(int)) && args(val) && if()")
   public static boolean test3(int val) {//use of formal binding is
straightforward
       return val + I > 0;
   }

   @Before("test()")
   public void doBefore() {
       System.out.println("IfAspect.doBefore");
   }

   @Before("test2()")
   public void doBefore2() {
       System.out.println("IfAspect.doBefore2");
   }

   @Before("execution(* Foo.do()) && if()") // enclosingDef is not a
@Pointcut returning boolean, with public static access
   public void doBefore3() {
       System.out.println("NO WAY - compiler error");
   }
}

  ReplyReply to allForward
		
		
Adrian Colyer 	
to me, Andrew, Jonas
	 More options	  11:25 am (5 hours ago)

I like the idea of the method containing the body of the if() pointcut - very
elegant!  I don't think we want to make the if() completely implicit, so:

@Pointcut("execution(* Foo.*(..))")
public static boolean fooThings() {
  return true;
}

should be an error, and

@Pointcut("execution(* Foo.*(..)) && if(true)")
public void fooThings() {}

would also be an error (not allowed to specify body for if() pointcut in
annotation style),

but

@Pointcut("execution(* Foo.*(..)) && if()")
public static boolean fooThings() {
  return true;
}

is great!

-- Adrian
Adrian_Colyer@uk.ibm.com


Alexandre Vasseur <avasseur@gmail.com>

08/06/2005 10:02
Please respond to
Alexandre Vasseur <avasseur@gmail.com>

	
To
	Adrian Colyer/UK/IBM@IBMGB, Andrew Clement/UK/IBM@IBMGB
cc
	Jonas Bonér <jboner@gmail.com>
Subject
	if pointcut in @AJ

	

- Show quoted text -



Here are my thought on the if() pointcut for @AJ stuff.
I don't think it is reasonable to have string based if(....) bodies so
here is the proposal I am starting on - the main idea is : the
@Pointcut annotated method acts as the if body (hence the if body is
already at hand etc etc)

so simple !
Alex

// if() is somehow implicit ie we can know we have an if when we have
// a static boolean @Pointcut annotatated method
// TODO TBD -may help toString(), parsing and tools if we require the
if() marker
@Aspect
public class IfAspect {

   static int I;

   @Pointcut("execution(* Foo.do()) && if()") // if(IfAspect.test())
   public static boolean test() {
       return I > 0;
   }

   @Pointcut("execution(* Foo.do()) && if()") //
if(IfAspect.test2(thisJoinPoint)
   public static boolean test2(JoinPoint jp) {// will require JP flags
       return jp.getSignature().getName().startsWith("baz") && I > 0;
   }

   @Pointcut("execution(* Foo.do(int)) && args(val) && if()")
   public static boolean test3(int val) {//use of formal binding is
straightforward
       return val + I > 0;
   }

   @Before("test()")
   public void doBefore() {
       System.out.println("IfAspect.doBefore");
   }

   @Before("test2()")
   public void doBefore2() {
       System.out.println("IfAspect.doBefore2");
   }

   @Before("execution(* Foo.do()) && if()") // enclosingDef is not a
@Pointcut returning boolean, with public static access
   public void doBefore3() {
       System.out.println("NO WAY - compiler error");
   }
}


  ReplyReply to allForwardInvite Adrian to Gmail
		
		
Alexandre Vasseur 	
to Adrian, Andrew, Jonas
	 More options	  1:52 pm (2½ hours ago)
Great - I reach the same "if() is required" conclusion
that was one of the last burden we had to go thru in terms of
semantics equivalence.

There is one implicit consequence: an anonymous pointcut
(@Before(...)) cannot contain an if() in the @AJ style. That's the
small special case to pay...

Alex
- Show quoted text -

On 6/8/05, Adrian Colyer <adrian_colyer@uk.ibm.com> wrote:
>
> I like the idea of the method containing the body of the if() pointcut -
> very elegant!  I don't think we want to make the if() completely implicit,
> so:
>
> @Pointcut("execution(* Foo.*(..))")
> public static boolean fooThings() {
>   return true;
> }
>
> should be an error, and
>
> @Pointcut("execution(* Foo.*(..)) && if(true)")
> public void fooThings() {}
>
> would also be an error (not allowed to specify body for if() pointcut in
> annotation style),
>
> but
>
> @Pointcut("execution(* Foo.*(..)) && if()")
> public static boolean fooThings() {
>   return true;
> }
>
> is great!
>
> -- Adrian
>  Adrian_Colyer@uk.ibm.com
>
>
>
>  Alexandre Vasseur <avasseur@gmail.com>
>
> 08/06/2005 10:02
>
> Please respond to
>  Alexandre Vasseur <avasseur@gmail.com>
>
>
> To Adrian Colyer/UK/IBM@IBMGB, Andrew Clement/UK/IBM@IBMGB
>
> cc Jonas Bonér <jboner@gmail.com>
>
> Subject if pointcut in @AJ
>
>
>
>
>
> Here are my thought on the if() pointcut for @AJ stuff.
>  I don't think it is reasonable to have string based if(....) bodies so
>  here is the proposal I am starting on - the main idea is : the
>  @Pointcut annotated method acts as the if body (hence the if body is
>  already at hand etc etc)
>
>  so simple !
>  Alex
>
>  // if() is somehow implicit ie we can know we have an if when we have
>  // a static boolean @Pointcut annotatated method
>  // TODO TBD -may help toString(), parsing and tools if we require the
>  if() marker
>  @Aspect
>  public class IfAspect {
>
>     static int I;
>
>     @Pointcut("execution(* Foo.do()) && if()") // if(IfAspect.test())
>     public static boolean test() {
>         return I > 0;
>     }
>
>     @Pointcut("execution(* Foo.do()) && if()") //
>  if(IfAspect.test2(thisJoinPoint)
>     public static boolean test2(JoinPoint jp) {// will require JP flags
>         return jp.getSignature().getName().startsWith("baz") && I > 0;
>     }
>
>     @Pointcut("execution(* Foo.do(int)) && args(val) && if()")
>     public static boolean test3(int val) {//use of formal binding is
>  straightforward
>         return val + I > 0;
>     }
>
>     @Before("test()")
>     public void doBefore() {
>         System.out.println("IfAspect.doBefore");
>     }
>
>     @Before("test2()")
>     public void doBefore2() {
>         System.out.println("IfAspect.doBefore2");
>     }
>
>     @Before("execution(* Foo.do()) && if()") // enclosingDef is not a
>  @Pointcut returning boolean, with public static access
>     public void doBefore3() {
>         System.out.println("NO WAY - compiler error");
>     }
>  }
>
>

  ReplyReply to allForward
		
		
Alexandre Vasseur 	
to Adrian
	 More options	  4:12 pm (15 minutes ago)
I actually like that a lot:

   @Pointcut("if()")
   public static boolean testONE(JoinPoint jp) {
       System.out.println("\tIfAspect.testONE");
       return jp.getSignature().getName().startsWith("do");
   }

ie a simple method gets turned into a full blown join point testing
library using regular Java
Comment 1 Alexandre Vasseur CLA 2005-06-08 10:35:30 EDT
Some short note to recap' on the rules:

"if()" can only appear in a @Pointcut(..) expression, and appears as is (no
if(....)). The such annotated method that usually is a "public void
<name>(<formals>) {}" must then be a "public static boolean <name>(<formal ||
implicit JoinPoint stuff>) { <test body> }"

Pointcut composition happens as usual ie formal AND implicit JoinPoint stuff
must be then bounded:

@Pointcut("if() && args(i)")
public static boolean test(JoinPoint jp, int i, JoinPoint.EnclosingStaticPart
esjp) {
   //... see implicit jp and esjp but explicit i
   return true;
}

@Before("test(jp, val, enc) && someOtherPointcut()") // all bound
public void beforeStuff(JoinPoint jp, JoinPoint.Enclosin.. enc, int val) {
   ...
}

Comment 2 Alexandre Vasseur CLA 2005-06-09 08:27:43 EDT
assigned to Adrian since some work for AJ-JDT stuff:

1/ in IfPointcut, testmethod can be null sometime (at least a NPE happens now).
The important thing is that I handle the IfPointcut as for @style IF the
extraParamFlag == -1. So testMethod can perhaps be there (that would perhaps
save a lookup if it is there - but then it must be the user if body and not the
ajc_if_xxx one..)

2/ language rules have changed a bit (see below)

To work on it use the AtAjSyntaxTest, testIf method, and uncomment in syntax.xml
where there is a    <!-- FIXME AV - Adrian in JDT stuff --> note.


Thanks
/ (I wish I could do that myself - if so, just tell me if I should and what is
the process, where is the readme to do it etc etc)

D:\java\jdk1.5.0_01\bin\java -Didea.launcher.port=7534
"-Didea.launcher.library=D:\Program Files\IntelliJ-IDEA-4.5\bin\breakgen.dll"
-Dfile.encoding=windows-1252 -classpath
"D:\java\jdk1.5.0_01\jre\lib\charsets.jar;D:\java\jdk1.5.0_01\jre\lib\deploy.jar;D:\java\jdk1.5.0_01\jre\lib\javaws.jar;D:\java\jdk1.5.0_01\jre\lib\jce.jar;D:\java\jdk1.5.0_01\jre\lib\jsse.jar;D:\java\jdk1.5.0_01\jre\lib\plugin.jar;D:\java\jdk1.5.0_01\jre\lib\rt.jar;D:\java\jdk1.5.0_01\jre\lib\ext\dnsns.jar;D:\java\jdk1.5.0_01\jre\lib\ext\localedata.jar;D:\java\jdk1.5.0_01\jre\lib\ext\sunjce_provider.jar;D:\java\jdk1.5.0_01\jre\lib\ext\sunpkcs11.jar;D:\aw\cvs_aj\org.aspectj-HEAD\modules\tests\bin;D:\aw\cvs_aj\org.aspectj-HEAD\modules\testing-drivers\bin;D:\aw\cvs_aj\org.aspectj-HEAD\modules\bridge\bin;D:\aw\cvs_aj\org.aspectj-HEAD\modules\lib\junit\junit.jar;D:\aw\cvs_aj\org.aspectj-HEAD\modules\util\bin;D:\aw\cvs_aj\org.aspectj-HEAD\modules\weaver\bin;D:\aw\cvs_aj\org.aspectj-HEAD\modules\runtime\bin;D:\aw\cvs_aj\org.aspectj-HEAD\modules\bcel-builder\bin;D:\aw\cvs_aj\org.aspectj-HEAD\modules\lib\regexp\jakarta-regexp-1.2.jar;D:\aw\cvs_aj\org.aspectj-HEAD\modules\testing-util\bin;D:\aw\cvs_aj\org.aspectj-HEAD\modules\lib\jdiff\jdiff.jar;D:\aw\cvs_aj\org.aspectj-HEAD\modules\asm\bin;D:\aw\cvs_aj\org.aspectj-HEAD\modules\testing\bin;D:\aw\cvs_aj\org.aspectj-HEAD\modules\lib\ant\lib\ant.jar;D:\aw\cvs_aj\org.aspectj-HEAD\modules\lib\ant\lib\ant-launcher.jar;D:\aw\cvs_aj\org.aspectj-HEAD\modules\testing-client\bin;D:\aw\cvs_aj\org.aspectj-HEAD\modules\ajde\bin;D:\aw\cvs_aj\org.aspectj-HEAD\modules\org.aspectj.ajdt.core\bin;D:\aw\cvs_aj\org.aspectj-HEAD\modules\org.eclipse.jdt.core\jdtcore-for-aspectj.jar;D:\aw\cvs_aj\org.aspectj-HEAD\modules\taskdefs\bin;D:\aw\cvs_aj\org.aspectj-HEAD\modules\lib\commons\commons.jar;D:\java\jdk1.5.0_01\lib\tools.jar;D:\aw\cvs_aj\org.aspectj-HEAD\modules\loadtime\bin;D:\aw\cvs_aj\org.aspectj-HEAD\modules\aspectj5rt\bin;D:\aw\cvs_aj\org.aspectj-HEAD\modules\loadtime5\bin;D:\Program
Files\IntelliJ-IDEA-4.5\lib\idea_rt.jar"
com.intellij.rt.execution.application.AppMain
com.intellij.rt.execution.junit2.JUnitStarter -ideVersion5
org.aspectj.systemtest.ajc150.ataspectj.AtAjSyntaxTests,testIfPointcut
LOADING SUITE: ..\tests\src\org\aspectj\systemtest\ajc150\ataspectj\syntax.xml
TEST: IfPointcutTest	...test "IfPointcutTest" failed
Unexpected error messages:
	error at (no source information available)
C:\Temp\ajcSandbox\ajcTest26370.tmp\ataspectj\IfPointcutTest.java:0::0 Internal
compiler error
java.lang.NullPointerException
	at org.aspectj.weaver.patterns.IfPointcut.write(IfPointcut.java:120)
	at org.aspectj.weaver.patterns.AndPointcut.write(AndPointcut.java:119)
	at
org.aspectj.weaver.ResolvedPointcutDefinition.write(ResolvedPointcutDefinition.java:72)
	at
org.aspectj.weaver.AjAttribute$PointcutDeclarationAttribute.write(AjAttribute.java:349)
	at org.aspectj.weaver.AjAttribute.getBytes(AjAttribute.java:57)
	at org.aspectj.weaver.AjAttribute.getAllBytes(AjAttribute.java:70)
	at
org.aspectj.ajdt.internal.compiler.ast.EclipseAttributeAdapter.getAllBytes(EclipseAttributeAdapter.java:31)
	at
org.aspectj.org.eclipse.jdt.internal.compiler.ClassFile.addAttributes(ClassFile.java:676)
	at
org.aspectj.org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.generateAttributes(TypeDeclaration.java:594)
	at
org.aspectj.org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.generateCode(TypeDeclaration.java:578)
	at
org.aspectj.org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.generateCode(TypeDeclaration.java:620)
	at
org.aspectj.org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.generateCode(TypeDeclaration.java:560)
	at
org.aspectj.org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.generateCode(TypeDeclaration.java:628)
	at
org.aspectj.org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration.generateCode(CompilationUnitDeclaration.java:182)
	at
org.aspectj.org.eclipse.jdt.internal.compiler.Compiler.process(Compiler.java:513)
	at
org.aspectj.org.eclipse.jdt.internal.compiler.Compiler.compile(Compiler.java:330)
	at
org.aspectj.ajdt.internal.core.builder.AjBuildManager.performCompilation(AjBuildManager.java:727)
	at
org.aspectj.ajdt.internal.core.builder.AjBuildManager.doBuild(AjBuildManager.java:206)

...
	at java.lang.reflect.Method.invoke(Method.java:585)
	at com.intellij.rt.execution.application.AppMain.main(AppMain.java:78)

	error at public static boolean positive(int i) {
              
C:\Temp\ajcSandbox\ajcTest26370.tmp\ataspectj\IfPointcutTest.java:57:0::0
Pointcuts should have an empty method body
	error at public static boolean positive(int i) {
              
C:\Temp\ajcSandbox\ajcTest26370.tmp\ataspectj\IfPointcutTest.java:57:0::0
Methods annotated with @Pointcut must return void
Unexpected fail messages:
	abort ABORT -- (NullPointerException) null
null
Comment 3 Adrian Colyer CLA 2005-06-09 08:43:34 EDT
I'll look into this shortly - and also write down any process hints for what has
to be done in situations like this...
Comment 4 Alexandre Vasseur CLA 2005-07-05 05:45:03 EDT
reassign to Adrian for JDT fix
Comment 5 Adrian Colyer CLA 2005-08-15 12:56:49 EDT
fixed in ValidateAtAspectJAnnotationsVisitor and IfPointcut. When I enable the
referenced tests, they still fail but with formal unbound in pointcut errors. Is
this expected?? If not we can work some more on this tomorrow.
Comment 6 Alexandre Vasseur CLA 2005-08-16 11:22:33 EDT
Yes IfPointcut and IfPointcut2 should pass.

They pass when -Xdev:NoAtAspectJProcessing is used (see syntax.xml).

When I uncomment the FIXME in syntax.xml :
The first one complains with a null body for "positive" method but the method
does have a body.
The second one complains with a formal unbound, but all is bound (and the bcel
weaver when -Xdev:NoAtAspectJProcessing is used does not complains).
Comment 7 Adrian Colyer CLA 2005-08-17 03:42:14 EDT
ok, will look into it this morning then.
Comment 8 Adrian Colyer CLA 2005-08-17 04:26:03 EDT
Problem with IfPointcutTest was that when looking for the matching method
containing the test, we iterated over getDeclaredJavaMethods. When the @aspect
was compiled by ajc, this isn't quite right as the @pointcut method becomes
treated like a pointcut declaration. Fix was just to iterate over getMethods.

IfPointcut2Test exercised a part of ValidateAtAspectJAnnotationsVisitor that we
hadn't gone through before. This was missing the logic to create
**Implicit**FormalBindings instead of regular FormalBindings for our special types.

Both tests are now passing for me.
I'm just running the full suite and then I'll commit if everything goes ok.
Comment 9 Adrian Colyer CLA 2005-08-17 04:41:37 EDT
ok, fix is committed.
Comment 10 Alexandre Vasseur CLA 2005-08-17 04:55:23 EDT
great! I could not figure out those details in order to fix that one.
Comment 11 Alexandre Vasseur CLA 2005-09-27 09:27:11 EDT
was M3 remind
Comment 12 Alexandre Vasseur CLA 2005-09-27 09:28:39 EDT
was M3 remind