From 6cceb1b9c3b53e92166d61435f28318e2b9a8872 Mon Sep 17 00:00:00 2001 From: jhugunin Date: Wed, 28 Jan 2004 00:36:05 +0000 Subject: [PATCH] fix for Bugzilla Bug 49295 duplicate warning or second join point for constructor-execution --- .../ast/InterTypeConstructorDeclaration.java | 2 + tests/ajcTests.xml | 17 ++++ tests/ajcTestsFailing.xml | 16 ++-- tests/bugs/InterfaceInitializerOrder.java | 26 ++++++ tests/bugs/SubtypeConstructorCW.java | 4 +- ...terAdviceOnConstructorsOnTheWrongType.java | 4 +- tests/new/AfterReturningConstructor.java | 8 +- .../AfterReturningInterfaceConstructor.java | 11 ++- .../AfterReturningInterfaceConstructorCE.java | 33 ++++++++ tests/new/EmptyInterface.java | 2 +- tests/new/InitializerTest.java | 10 +-- tests/new/NegativeSourceLocation.java | 4 +- .../aspectj/weaver/bcel/BcelClassWeaver.java | 50 +++++------- .../org/aspectj/weaver/bcel/BcelShadow.java | 80 +++++++++++-------- 14 files changed, 172 insertions(+), 95 deletions(-) create mode 100644 tests/bugs/InterfaceInitializerOrder.java create mode 100644 tests/new/AfterReturningInterfaceConstructorCE.java diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeConstructorDeclaration.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeConstructorDeclaration.java index 0f189c34c..972d07c2a 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeConstructorDeclaration.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeConstructorDeclaration.java @@ -194,6 +194,8 @@ public class InterTypeConstructorDeclaration extends InterTypeDeclaration { if (onTypeBinding.isInterface()) { + classScope.problemReporter().signalError(sourceStart, sourceEnd, + "can't define constructors on interfaces"); ignoreFurtherInvestigation = true; return null; } diff --git a/tests/ajcTests.xml b/tests/ajcTests.xml index a853bef28..8d17bd6fe 100644 --- a/tests/ajcTests.xml +++ b/tests/ajcTests.xml @@ -5197,6 +5197,13 @@ + + + + + + @@ -7119,4 +7126,14 @@ + + + + + + + + diff --git a/tests/ajcTestsFailing.xml b/tests/ajcTestsFailing.xml index dca686139..a64b4fb3c 100644 --- a/tests/ajcTestsFailing.xml +++ b/tests/ajcTestsFailing.xml @@ -127,16 +127,6 @@ - - - - - - - - @@ -160,4 +150,10 @@ + + + + + diff --git a/tests/bugs/InterfaceInitializerOrder.java b/tests/bugs/InterfaceInitializerOrder.java new file mode 100644 index 000000000..3036bcac9 --- /dev/null +++ b/tests/bugs/InterfaceInitializerOrder.java @@ -0,0 +1,26 @@ +import org.aspectj.testing.Tester; + +public class InterfaceInitializerOrder { + public static void main(String[] args) { + Base base = new Base(); + Tester.checkEqual(InitAspect.inits.toString(), "Super1,Super2,SuperInterface,Base,"); + } +} + +class Super1 {} + +class Super2 extends Super1 {} + +interface SuperInterface {} + +class Base extends Super2 implements SuperInterface { } + +aspect InitAspect { + public static StringBuffer inits = new StringBuffer(); + + pointcut outerMatch() : initialization(new(..)) && !within(InitAspect); + before() : outerMatch() { + inits.append(thisJoinPoint.getSignature().getDeclaringType().getName()); + inits.append(","); + } +} diff --git a/tests/bugs/SubtypeConstructorCW.java b/tests/bugs/SubtypeConstructorCW.java index d3b6a8765..f37ac8b37 100644 --- a/tests/bugs/SubtypeConstructorCW.java +++ b/tests/bugs/SubtypeConstructorCW.java @@ -7,8 +7,8 @@ class C implements Runnable { // CW 5 } } class F implements Runnable { - F(int i) {// CW 10 - } + F(int i) {}// CW 10 + public void run() { } } diff --git a/tests/new/AfterAdviceOnConstructorsOnTheWrongType.java b/tests/new/AfterAdviceOnConstructorsOnTheWrongType.java index e29ee974a..21ab6cb6b 100644 --- a/tests/new/AfterAdviceOnConstructorsOnTheWrongType.java +++ b/tests/new/AfterAdviceOnConstructorsOnTheWrongType.java @@ -10,8 +10,8 @@ public class AfterAdviceOnConstructorsOnTheWrongType { } static { Tester.clearEvents(); - // new(..) for both class and interface - Tester.expectEventsInString("after-c,after-c,c,after-d,after-d,d"); + // new(..) for just class + Tester.expectEventsInString("after-c,c,after-d,d"); } } diff --git a/tests/new/AfterReturningConstructor.java b/tests/new/AfterReturningConstructor.java index e0fad5e0e..3d87758e5 100644 --- a/tests/new/AfterReturningConstructor.java +++ b/tests/new/AfterReturningConstructor.java @@ -32,8 +32,8 @@ class U { static final String afterThrowing = "after() throwing(): "; static final String c = "C()"; static final String i = "I()"; - static final String cjp = "execution(" + c + ")"; - static final String ijp = "execution(" + i + ")"; + static final String cjp = "initialization(" + c + ")"; + static final String ijp = "initialization(" + i + ")"; static void e(String event) { //System.err.println("act event: " + event); // XXX @@ -46,8 +46,8 @@ class U { } aspect A { - /** must pick out both interface and implementor constructor execution */ - pointcut pc(): execution(new(..)) && !within(A); + /** must pick out both interface and implementor initializers */ + pointcut pc(): initialization(new(..)) && !within(A); before(): pc() { U.e(U.before + thisJoinPoint); diff --git a/tests/new/AfterReturningInterfaceConstructor.java b/tests/new/AfterReturningInterfaceConstructor.java index 71e851867..f16f563bf 100644 --- a/tests/new/AfterReturningInterfaceConstructor.java +++ b/tests/new/AfterReturningInterfaceConstructor.java @@ -6,7 +6,10 @@ public class AfterReturningInterfaceConstructor { public static void main (String[] args) { Tester.expectEvent("constructor"); Tester.expectEvent("advice"); - new C(); + I i = new C(); + + Tester.checkEqual(i.i, 2, "i.i"); + Tester.checkAllEvents(); } } @@ -21,10 +24,10 @@ class C implements I { aspect A { int I.i; - I.new() { - i = 2; + after(I i) returning: this(i) && initialization(I.new(..)) { + i.i = 2; } - after() returning: execution(I.new()) { + after() returning: initialization(I.new(..)) { Tester.event("advice"); } } diff --git a/tests/new/AfterReturningInterfaceConstructorCE.java b/tests/new/AfterReturningInterfaceConstructorCE.java new file mode 100644 index 000000000..aec297d2c --- /dev/null +++ b/tests/new/AfterReturningInterfaceConstructorCE.java @@ -0,0 +1,33 @@ + +import org.aspectj.testing.Tester; + +/** @testcase PR#889 after returning advice on interface constructor */ +public class AfterReturningInterfaceConstructorCE { + public static void main (String[] args) { + Tester.expectEvent("constructor"); + Tester.expectEvent("advice"); + I i = new C(); + System.out.println("i.i: " + i.i); + + Tester.checkAllEvents(); + } +} + +interface I {} + +class C implements I { + C() { + Tester.event("constructor"); + } +} + +aspect A { + int I.i; + I.new() { + i = 2; + System.out.println("running I.new()"); + } + after() returning: execution(I.new()) { // ERR: can't define constructor on interface + Tester.event("advice"); + } +} diff --git a/tests/new/EmptyInterface.java b/tests/new/EmptyInterface.java index aca01b6a8..396f7994c 100644 --- a/tests/new/EmptyInterface.java +++ b/tests/new/EmptyInterface.java @@ -20,7 +20,7 @@ aspect Log { interface LoggedType { } declare parents: C implements LoggedType; - after() : within(LoggedType+) + after(): within(LoggedType+) //&& !initialization(new(..)) //&& !preinitialization(new(..)) // 1.1 only { diff --git a/tests/new/InitializerTest.java b/tests/new/InitializerTest.java index 52f2d93af..31b5ca698 100644 --- a/tests/new/InitializerTest.java +++ b/tests/new/InitializerTest.java @@ -81,10 +81,10 @@ aspect A issingleton () { before(I i): initialization(I.new()) && this(i) { Tester.checkEqual(((C)i).state, "C-constructed", thisJoinPoint.toString()); } - before(I i): execution(I.new()) && this(i) { - Tester.checkEqual(((C)i).state, "C-constructed", thisJoinPoint.toString()); - Tester.note("constructed I"); - } +// before(I i): execution(I.new()) && this(i) { +// Tester.checkEqual(((C)i).state, "C-constructed", thisJoinPoint.toString()); +// Tester.note("constructed I"); +// } after(I i): initialization(I.new()) && this(i) { Tester.checkEqual(((C)i).state, "C-constructed", thisJoinPoint.toString()); Tester.note("initialized I"); @@ -100,7 +100,7 @@ public class InitializerTest { Tester.check("constructed SubC"); Tester.check("initialized I"); - Tester.check("constructed I"); + //Tester.check("constructed I"); Tester.check("static initialized C"); Tester.check("static initialized SubC"); diff --git a/tests/new/NegativeSourceLocation.java b/tests/new/NegativeSourceLocation.java index f76ef229a..f27bf3925 100644 --- a/tests/new/NegativeSourceLocation.java +++ b/tests/new/NegativeSourceLocation.java @@ -287,7 +287,7 @@ class Const { , "before AllTargetJoinPoints set(String TargetClass.staticString)" , "before AllTargetJoinPoints preinitialization(TargetClass())" , "before AllTargetJoinPoints initialization(java.lang.Runnable())" - , "before AllTargetJoinPoints execution(java.lang.Runnable())" + //, "before AllTargetJoinPoints execution(java.lang.Runnable())" , "before AllTargetJoinPoints initialization(TargetClass())" //, "before AllTargetJoinPoints execution(TargetClass.)" , "before AllTargetJoinPoints set(String TargetClass.string)" @@ -305,7 +305,7 @@ class Const { , "before AllTargetJoinPoints preinitialization(TargetClass())" , "before AllTargetJoinPoints initialization(TargetClass())" , "before AllTargetJoinPoints initialization(java.lang.Runnable())" - , "before AllTargetJoinPoints execution(java.lang.Runnable())" + //, "before AllTargetJoinPoints execution(java.lang.Runnable())" //, "before AllTargetJoinPoints execution(TargetClass.)" , "before AllTargetJoinPoints set(String TargetClass.string)" , "before AllTargetJoinPoints execution(TargetClass())" diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java index 052cef1fa..d67d4eb87 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java @@ -391,7 +391,7 @@ class BcelClassWeaver implements IClassWeaver { Range.genStart(body, body.getStart().getNext()), Range.genEnd(body, call.getPrev())); } else { - // assert s.getKind() == Shadow.Initialization + // assert s.getKind() == Shadow.Initialization r.associateWithTargets( Range.genStart(body, call.getNext()), Range.genEnd(body)); @@ -748,50 +748,40 @@ class BcelClassWeaver implements IClassWeaver { // walk the body boolean beforeSuperOrThisCall = true; - if (shouldWeaveBody(mg)) { //!mg.isAjSynthetic()) { - for (InstructionHandle h = mg.getBody().getStart(); - h != null; - h = h.getNext()) { - if (h == superOrThisCall) { - beforeSuperOrThisCall = false; - continue; + if (shouldWeaveBody(mg)) { + if (canMatchBodyShadows) { + for (InstructionHandle h = mg.getBody().getStart(); + h != null; + h = h.getNext()) { + if (h == superOrThisCall) { + beforeSuperOrThisCall = false; + continue; + } + match(mg, h, beforeSuperOrThisCall ? null : enclosingShadow, shadowAccumulator); } - match(mg, h, beforeSuperOrThisCall ? null : enclosingShadow, shadowAccumulator); } match(enclosingShadow, shadowAccumulator); } // XXX we don't do pre-inits of interfaces - // now add interface inits and cexecs + // now add interface inits if (superOrThisCall != null && ! isThisCall(superOrThisCall)) { InstructionHandle curr = enclosingShadow.getRange().getStart(); for (Iterator i = addedSuperInitializersAsList.iterator(); i.hasNext(); ) { IfaceInitList l = (IfaceInitList) i.next(); - // generate the cexec jp + Member ifaceInitSig = AjcMemberMaker.interfaceConstructor(l.onType); - BcelShadow cexecShadow = - BcelShadow.makeIfaceConstructorExecution( - world, - mg, - curr, - ifaceInitSig); - if (match(cexecShadow, shadowAccumulator)) { - cexecShadow.getRange().getBody().append( - cexecShadow.getRange().getStart(), - InstructionConstants.NOP); - } - // generate the init jp around it + BcelShadow initShadow = - BcelShadow.makeIfaceInitialization( - world, - mg, - cexecShadow, - ifaceInitSig); - match(initShadow, shadowAccumulator); + BcelShadow.makeIfaceInitialization(world, mg, ifaceInitSig); + // insert code in place InstructionList inits = genInitInstructions(l.list, false); - initShadow.getRange().insert(inits, Range.InsideAfter); + if (match(initShadow, shadowAccumulator) || !inits.isEmpty()) { + initShadow.initIfaceInitializer(curr); + initShadow.getRange().insert(inits, Range.OutsideBefore); + } } // now we add our initialization code diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java index 41a84491a..fd72970cd 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java @@ -430,7 +430,6 @@ public class BcelShadow extends Shadow { public static BcelShadow makeIfaceInitialization( BcelWorld world, LazyMethodGen constructor, - BcelShadow ifaceCExecShadow, Member interfaceConstructorSignature) { InstructionList body = constructor.getBody(); @@ -443,44 +442,55 @@ public class BcelShadow extends Shadow { constructor, null); s.fallsThrough = true; - ShadowRange r = new ShadowRange(body); - r.associateWithShadow(s); - InstructionHandle start = Range.genStart(body, ifaceCExecShadow.getRange().getStart()); - InstructionHandle end = Range.genEnd(body, ifaceCExecShadow.getRange().getEnd()); - - r.associateWithTargets(start, end); +// ShadowRange r = new ShadowRange(body); +// r.associateWithShadow(s); +// InstructionHandle start = Range.genStart(body, handle); +// InstructionHandle end = Range.genEnd(body, handle); +// +// r.associateWithTargets(start, end); return s; } - - public static BcelShadow makeIfaceConstructorExecution( - BcelWorld world, - LazyMethodGen constructor, - InstructionHandle next, - Member interfaceConstructorSignature) - { - // final InstructionFactory fact = constructor.getEnclosingClass().getFactory(); - InstructionList body = constructor.getBody(); - // TypeX inType = constructor.getEnclosingClass().getType(); - BcelShadow s = - new BcelShadow( - world, - ConstructorExecution, - interfaceConstructorSignature, - constructor, - null); - s.fallsThrough = true; - ShadowRange r = new ShadowRange(body); - r.associateWithShadow(s); - // ??? this may or may not work - InstructionHandle start = Range.genStart(body, next); - //InstructionHandle end = Range.genEnd(body, body.append(start, fact.NOP)); - InstructionHandle end = Range.genStart(body, next); - //body.append(start, fact.NOP); - - r.associateWithTargets(start, end); - return s; + + public void initIfaceInitializer(InstructionHandle end) { + final InstructionList body = enclosingMethod.getBody(); + ShadowRange r = new ShadowRange(body); + r.associateWithShadow(this); + InstructionHandle nop = body.insert(end, InstructionConstants.NOP); + + r.associateWithTargets( + Range.genStart(body, nop), + Range.genEnd(body, nop)); } +// public static BcelShadow makeIfaceConstructorExecution( +// BcelWorld world, +// LazyMethodGen constructor, +// InstructionHandle next, +// Member interfaceConstructorSignature) +// { +// // final InstructionFactory fact = constructor.getEnclosingClass().getFactory(); +// InstructionList body = constructor.getBody(); +// // TypeX inType = constructor.getEnclosingClass().getType(); +// BcelShadow s = +// new BcelShadow( +// world, +// ConstructorExecution, +// interfaceConstructorSignature, +// constructor, +// null); +// s.fallsThrough = true; +// ShadowRange r = new ShadowRange(body); +// r.associateWithShadow(s); +// // ??? this may or may not work +// InstructionHandle start = Range.genStart(body, next); +// //InstructionHandle end = Range.genEnd(body, body.append(start, fact.NOP)); +// InstructionHandle end = Range.genStart(body, next); +// //body.append(start, fact.NOP); +// +// r.associateWithTargets(start, end); +// return s; +// } + /** Create an initialization join point associated with a constructor, but not * with any body of code yet. If this is actually matched, it's range will be set -- 2.39.5