From 1e28b926584c406e4822ec153f6bac07af47771d Mon Sep 17 00:00:00 2001 From: aclement Date: Tue, 25 May 2010 23:03:13 +0000 Subject: [PATCH] 314365: reliable rewriting --- .../PointcutEvaluationExpenseComparator.java | 28 ++++++++++++---- .../weaver/patterns/PointcutRewriterTest.java | 33 ++++++++++++++++--- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/patterns/PointcutEvaluationExpenseComparator.java b/org.aspectj.matcher/src/org/aspectj/weaver/patterns/PointcutEvaluationExpenseComparator.java index 2fb9321f9..1d2643261 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/patterns/PointcutEvaluationExpenseComparator.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/patterns/PointcutEvaluationExpenseComparator.java @@ -47,7 +47,6 @@ public class PointcutEvaluationExpenseComparator implements Comparator * @args cflow, cflowbelow if */ public int compare(Pointcut p1, Pointcut p2) { - // important property for a well-defined comparator if (p1.equals(p2)) { return 0; @@ -56,9 +55,14 @@ public class PointcutEvaluationExpenseComparator implements Comparator if (result == 0) { // they have the same evaluation expense, but are not 'equal' // sort by hashCode - result = p1.hashCode() - p2.hashCode(); - if (result == 0) { - /* not allowed if ne */return -1; + int p1code = p1.hashCode(); + int p2code = p2.hashCode(); + if (p1code == p2code) { + return 0; + } else if (p1code < p2code) { + return -1; + } else { + return +1; } } return result; @@ -129,10 +133,22 @@ public class PointcutEvaluationExpenseComparator implements Comparator return getScore(((NotPointcut) p).getNegatedPointcut()); } if (p instanceof AndPointcut) { - return getScore(((AndPointcut) p).getLeft()); + int leftScore = getScore(((AndPointcut) p).getLeft()); + int rightScore = getScore(((AndPointcut) p).getRight()); + if (leftScore < rightScore) { + return leftScore; + } else { + return rightScore; + } } if (p instanceof OrPointcut) { - return getScore(((OrPointcut) p).getLeft()); + int leftScore = getScore(((OrPointcut) p).getLeft()); + int rightScore = getScore(((OrPointcut) p).getRight()); + if (leftScore > rightScore) { + return leftScore; + } else { + return rightScore; + } } return OTHER; } diff --git a/org.aspectj.matcher/testsrc/org/aspectj/weaver/patterns/PointcutRewriterTest.java b/org.aspectj.matcher/testsrc/org/aspectj/weaver/patterns/PointcutRewriterTest.java index 60b75b937..d2a8fd245 100644 --- a/org.aspectj.matcher/testsrc/org/aspectj/weaver/patterns/PointcutRewriterTest.java +++ b/org.aspectj.matcher/testsrc/org/aspectj/weaver/patterns/PointcutRewriterTest.java @@ -18,15 +18,39 @@ import junit.framework.TestCase; import org.aspectj.weaver.Shadow; /** - * @author colyer + * Testing the pointcut rewriter. * - * TODO To change the template for this generated type comment go to Window - Preferences - Java - Code Style - Code - * Templates + * @author Adrian Colyer + * @author Andy Clement */ public class PointcutRewriterTest extends TestCase { private PointcutRewriter prw; + public void testComplexRewrite1() { + Pointcut p = getPointcut("(persingleton(org.eclipse.ajdt.internal.ui.ras.UIFFDC) && ((handler(java.lang.Throwable+) && args(arg1)) && ((within(org.eclipse.ajdt..*) && (!within(org.eclipse.ajdt.internal.ui.lazystart..*) && (!within(org.eclipse.ajdt.internal.ui.dialogs.OpenTypeSelectionDialog2) && !(within(org.eclipse.ajdt.internal.ui.editor.AspectJBreakpointRulerAction) && handler(org.eclipse.jface.text.BadLocationException))))) && (!(within(org.eclipse.ajdt.core.ras.FFDC+) || handler(org.eclipse.core.runtime.OperationCanceledException)) && !this(java.lang.Object)))))"); + checkMultipleRewrite(p); + p = getPointcut("((((((((((!within(org.eclipse.ajdt.internal.ui.lazystart..*) && !within(org.eclipse.ajdt.internal.ui.dialogs.OpenTypeSelectionDialog2)) && !within(org.eclipse.ajdt.core.ras.FFDC+)) && within(org.eclipse.ajdt..*)) && !within(org.eclipse.ajdt.internal.ui.editor.AspectJBreakpointRulerAction)) && handler(java.lang.Throwable+)) && !handler(org.eclipse.core.runtime.OperationCanceledException)) && !this(java.lang.Object)) && args(arg1)) && persingleton(org.eclipse.ajdt.internal.ui.ras.UIFFDC)) || (((((((((!within(org.eclipse.ajdt.internal.ui.lazystart..*) && !within(org.eclipse.ajdt.internal.ui.dialogs.OpenTypeSelectionDialog2)) && !within(org.eclipse.ajdt.core.ras.FFDC+)) && within(org.eclipse.ajdt..*)) && !handler(org.eclipse.jface.text.BadLocationException)) && handler(java.lang.Throwable+)) && !handler(org.eclipse.core.runtime.OperationCanceledException)) && !this(java.lang.Object)) && args(arg1)) && persingleton(org.eclipse.ajdt.internal.ui.ras.UIFFDC)))"); + checkMultipleRewrite(p); + p = getPointcut("(persingleton(Oranges) && ((handler(Apples+) && args(arg1)) && ((within(foo..*) && (!within(org.eclipse.ajdt.internal.ui.lazystart..*) && (!within(org.eclipse.ajdt.internal.ui.dialogs.OpenTypeSelectionDialog2) && !(within(org.eclipse.ajdt.internal.ui.editor.AspectJBreakpointRulerAction) && handler(org.eclipse.jface.text.BadLocationException))))) && (!(within(org.eclipse.ajdt.core.ras.FFDC+) || handler(org.eclipse.core.runtime.OperationCanceledException)) && !this(java.lang.Object)))))"); + checkMultipleRewrite(p); + p = getPointcut("(((handler(Apples+)) && ((within(foo..*) && (!within(org.eclipse.ajdt.internal.ui.lazystart..*) && (!within(org.eclipse.ajdt.internal.ui.dialogs.OpenTypeSelectionDialog2) && !(within(org.eclipse.ajdt.internal.ui.editor.AspectJBreakpointRulerAction) && handler(org.eclipse.jface.text.BadLocationException))))) && (!(within(org.eclipse.ajdt.core.ras.FFDC+) || handler(org.eclipse.core.runtime.OperationCanceledException)) && !this(java.lang.Object)))))"); + checkMultipleRewrite(p); + p = getPointcut("within(xxx..*) && within(XXY) && within(org.eclipse.AspectJBreakpoint)"); + checkMultipleRewrite(p); + } + + /** + * Rewrites a pointcut twice and checks the format is stable + */ + private void checkMultipleRewrite(Pointcut p) { + Pointcut rewrittenPointcut = prw.rewrite(p, false); + String rewrite = rewrittenPointcut.toString(); + Pointcut rewriteOfRewrittenPointcut = prw.rewrite(rewrittenPointcut, false); + String rewriteOfRewritten = rewriteOfRewrittenPointcut.toString(); + assertEquals(rewrite, rewriteOfRewritten); + } + public void testDistributeNot() { Pointcut plain = getPointcut("this(Foo)"); assertEquals("Unchanged", plain, prw.rewrite(plain)); @@ -417,9 +441,10 @@ public class PointcutRewriterTest extends TestCase { public void testOrderingInAnd() { Pointcut bigLongPC = getPointcut("cflow(this(Foo)) && @args(X) && args(X) && @this(Foo) && @target(Boo) && this(Moo) && target(Boo) && @annotation(Moo) && @withincode(Boo) && withincode(new(..)) && set(* *)&& @within(Foo) && within(Foo)"); + checkMultipleRewrite(bigLongPC); Pointcut rewritten = prw.rewrite(bigLongPC); assertEquals( - "((((((((((((within(Foo) && @within(Foo)) && set(* *)) && withincode(new(..))) && @withincode(Boo)) && target(Boo)) && this(Moo)) && @annotation(Moo)) && @target(Boo)) && @this(Foo)) && args(X)) && @args(X)) && cflow(this(Foo)))", + "((((((((((((within(Foo) && @within(Foo)) && set(* *)) && withincode(new(..))) && @withincode(Boo)) && target(Boo)) && this(Moo)) && @annotation(Moo)) && @this(Foo)) && @target(Boo)) && args(X)) && @args(X)) && cflow(this(Foo)))", rewritten.toString()); } -- 2.39.5