From: aclement Date: Wed, 1 Dec 2004 18:00:32 +0000 (+0000) Subject: 72766: This is covariance support. only missing bit is for dynamic join point matchi... X-Git-Tag: Root_AspectJ5_Development~228 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=d2fb8b7a3122f1e8816cf888b02643d5b31f9c3b;p=aspectj.git 72766: This is covariance support. only missing bit is for dynamic join point matching (marked XXXAJ5 in the code) - thats not a 1.5.0M1 item I dont think... --- diff --git a/tests/java5/covariance/CovAspect01.aj b/tests/java5/covariance/CovAspect01.aj new file mode 100644 index 000000000..fa66dd0c1 --- /dev/null +++ b/tests/java5/covariance/CovAspect01.aj @@ -0,0 +1,10 @@ +aspect CovAspect01 { + + pointcut p(): call(* getCar()); + + before(): p() { + System.out.println("[call(* getCar()) matched on '"+thisJoinPoint+":"+thisJoinPoint.getSourceLocation()+"']"); + } + +} + diff --git a/tests/java5/covariance/CovAspect02.aj b/tests/java5/covariance/CovAspect02.aj new file mode 100644 index 000000000..6a2472a02 --- /dev/null +++ b/tests/java5/covariance/CovAspect02.aj @@ -0,0 +1,10 @@ +aspect CovAspect02 { + + pointcut p(): call(* Super.getCar()); + + before(): p() { + System.out.println("[call(* Super.getCar()) matched on '"+thisJoinPoint+":"+thisJoinPoint.getSourceLocation()+"']"); + } + +} + diff --git a/tests/java5/covariance/CovAspect03.aj b/tests/java5/covariance/CovAspect03.aj new file mode 100644 index 000000000..551c4b613 --- /dev/null +++ b/tests/java5/covariance/CovAspect03.aj @@ -0,0 +1,10 @@ +aspect CovAspect03 { + + pointcut p(): call(Car getCar()); + + before(): p() { + System.out.println("[call(Car getCar()) matched on '"+thisJoinPoint+":"+thisJoinPoint.getSourceLocation()+"']"); + } + +} + diff --git a/tests/java5/covariance/CovAspect04.aj b/tests/java5/covariance/CovAspect04.aj new file mode 100644 index 000000000..e8dd00de4 --- /dev/null +++ b/tests/java5/covariance/CovAspect04.aj @@ -0,0 +1,10 @@ +aspect CovAspect04 { + + pointcut p(): call(Car Super.getCar()); + + before(): p() { + System.out.println("[call(Car Super.getCar()) matched on '"+thisJoinPoint+":"+thisJoinPoint.getSourceLocation()+"']"); + } + +} + diff --git a/tests/java5/covariance/CovAspect05.aj b/tests/java5/covariance/CovAspect05.aj new file mode 100644 index 000000000..268b179b0 --- /dev/null +++ b/tests/java5/covariance/CovAspect05.aj @@ -0,0 +1,10 @@ +aspect CovAspect05 { + + pointcut p(): call(Car Super.getCar()); + + before(): p() { + System.out.println("[call(Car Super.getCar()) matched on '"+thisJoinPoint+":"+thisJoinPoint.getSourceLocation()+"']"); + } + +} + diff --git a/tests/java5/covariance/CovAspect06.aj b/tests/java5/covariance/CovAspect06.aj new file mode 100644 index 000000000..bb4961360 --- /dev/null +++ b/tests/java5/covariance/CovAspect06.aj @@ -0,0 +1,10 @@ +aspect CovAspect06 { + + pointcut p(): call(Car Sub.getCar()); + + before(): p() { + System.out.println("[call(Car Sub.getCar()) matched on '"+thisJoinPoint+":"+thisJoinPoint.getSourceLocation()+"']"); + } + +} + diff --git a/tests/java5/covariance/CovAspect07.aj b/tests/java5/covariance/CovAspect07.aj new file mode 100644 index 000000000..67a76825b --- /dev/null +++ b/tests/java5/covariance/CovAspect07.aj @@ -0,0 +1,10 @@ +aspect CovAspect07 { + + pointcut p(): call(Car+ Sub.getCar()); + + before(): p() { + System.out.println("[call(Car+ Sub.getCar()) matched on '"+thisJoinPoint+":"+thisJoinPoint.getSourceLocation()+"']"); + } + +} + diff --git a/tests/java5/covariance/CovAspect08.aj b/tests/java5/covariance/CovAspect08.aj new file mode 100644 index 000000000..5920981dc --- /dev/null +++ b/tests/java5/covariance/CovAspect08.aj @@ -0,0 +1,16 @@ +aspect CovAspect08 { + + pointcut p1(): call(FastCar getCar()); + + before(): p1() { + System.out.println("[call(FastCar getCar()) matched on '"+thisJoinPoint+":"+thisJoinPoint.getSourceLocation()+"']"); + } + + pointcut p2(): call(FastCar Sub.getCar()); + + before(): p2() { + System.out.println("[call(FastCar Sub.getCar()) matched on '"+thisJoinPoint+":"+thisJoinPoint.getSourceLocation()+"']"); + } + +} + diff --git a/tests/java5/covariance/CovAspect09.aj b/tests/java5/covariance/CovAspect09.aj new file mode 100644 index 000000000..6a3497048 --- /dev/null +++ b/tests/java5/covariance/CovAspect09.aj @@ -0,0 +1,10 @@ +aspect CovAspect09 { + + pointcut p1(): call(FastCar Super.getCar()); + + before(): p1() { + System.out.println("[call(FastCar Super.getCar()) matched on '"+thisJoinPoint+":"+thisJoinPoint.getSourceLocation()+"']"); + } + +} + diff --git a/tests/java5/covariance/CovAspect10.aj b/tests/java5/covariance/CovAspect10.aj new file mode 100644 index 000000000..158a3e1e2 --- /dev/null +++ b/tests/java5/covariance/CovAspect10.aj @@ -0,0 +1,10 @@ +aspect CovAspect10 { + + pointcut p1(): call(Car+ getCar()); + + before(): p1() { + System.out.println("[call(Car+ getCar()) matched on '"+thisJoinPoint+":"+thisJoinPoint.getSourceLocation()+"']"); + } + +} + diff --git a/tests/java5/covariance/CovAspect11.aj b/tests/java5/covariance/CovAspect11.aj new file mode 100644 index 000000000..19f742e85 --- /dev/null +++ b/tests/java5/covariance/CovAspect11.aj @@ -0,0 +1,10 @@ +aspect CovAspect11 { + + pointcut p1(): call(Car+ Sub.getCar()); + + before(): p1() { + System.out.println("[call(Car+ Sub.getCar()) matched on '"+thisJoinPoint+":"+thisJoinPoint.getSourceLocation()+"']"); + } + +} + diff --git a/tests/java5/covariance/CovBaseProgram01.jar b/tests/java5/covariance/CovBaseProgram01.jar new file mode 100644 index 000000000..5a89abea2 Binary files /dev/null and b/tests/java5/covariance/CovBaseProgram01.jar differ diff --git a/tests/java5/covariance/CovBaseProgram01.java b/tests/java5/covariance/CovBaseProgram01.java new file mode 100644 index 000000000..169a6be10 --- /dev/null +++ b/tests/java5/covariance/CovBaseProgram01.java @@ -0,0 +1,32 @@ +class Car {} + +class FastCar extends Car {} + +class Super { + Car getCar() { + return new Car(); + } +} + +class Sub extends Super { + FastCar getCar() { + return new FastCar(); + } +} + +public class CovBaseProgram01 { + public static void main(String[] argv) { + new CovBaseProgram01().run(); + } + + public void run() { + Super instance_super = new Super(); + Sub instance_sub = new Sub(); + + Car c1 = instance_super.getCar(); + Car c2 = instance_sub.getCar(); + } +} + +// FastCar is a subclass of Car. +// Sub is a subclass of Super. \ No newline at end of file diff --git a/tests/java5/covariance/CovBaseProgram02.jar b/tests/java5/covariance/CovBaseProgram02.jar new file mode 100644 index 000000000..93898a284 Binary files /dev/null and b/tests/java5/covariance/CovBaseProgram02.jar differ diff --git a/tests/java5/covariance/CovBaseProgram02.java b/tests/java5/covariance/CovBaseProgram02.java new file mode 100644 index 000000000..43f5a873a --- /dev/null +++ b/tests/java5/covariance/CovBaseProgram02.java @@ -0,0 +1,36 @@ +class Car { + Car() {} +} + +class FastCar extends Car { + FastCar() {} +} + +class Super { + Car getCar() { + return new Car(); + } +} + +class Sub { + FastCar getCar() { + return new FastCar(); + } +} + +public class CovBaseProgram02 { + public static void main(String[] argv) { + new CovBaseProgram02().run(); + } + + public void run() { + Super instance_super = new Super(); + Sub instance_sub = new Sub(); + + Car c1 = instance_super.getCar(); + FastCar c2 = instance_sub.getCar(); + } +} + +// Lemon is a subclass of Car +// Sub is *not* a subclass of Super diff --git a/tests/java5/covariance/build.xml b/tests/java5/covariance/build.xml new file mode 100644 index 000000000..121ec3b46 --- /dev/null +++ b/tests/java5/covariance/build.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + + + diff --git a/tests/src/org/aspectj/systemtest/ajc150/AllTestsJava5_binaryWeaving.java b/tests/src/org/aspectj/systemtest/ajc150/AllTestsJava5_binaryWeaving.java index c6b9d3787..8412a7516 100644 --- a/tests/src/org/aspectj/systemtest/ajc150/AllTestsJava5_binaryWeaving.java +++ b/tests/src/org/aspectj/systemtest/ajc150/AllTestsJava5_binaryWeaving.java @@ -26,6 +26,7 @@ public class AllTestsJava5_binaryWeaving { //$JUnit-BEGIN$ suite.addTest(Ajc150Tests.suite()); suite.addTest(AccBridgeMethods.suite()); + suite.addTestSuite(CovarianceTests.class); //$JUnit-END$ return suite; } diff --git a/tests/src/org/aspectj/systemtest/ajc150/CovarianceTests.java b/tests/src/org/aspectj/systemtest/ajc150/CovarianceTests.java new file mode 100644 index 000000000..120deee0f --- /dev/null +++ b/tests/src/org/aspectj/systemtest/ajc150/CovarianceTests.java @@ -0,0 +1,302 @@ +/******************************************************************************* + * Copyright (c) 2004 IBM Corporation and others. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Common Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/cpl-v10.html + * + * Contributors: + * Andy Clement - initial implementation + *******************************************************************************/ +package org.aspectj.systemtest.ajc150; + +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; +import java.util.Iterator; +import java.util.List; +import java.util.Set; + +import junit.framework.Test; + +import org.aspectj.bridge.IMessage; +import org.aspectj.testing.XMLBasedAjcTestCase; +import org.aspectj.tools.ajc.AjcTestCase; +import org.aspectj.tools.ajc.CompilationResult; + +/* + +class Car {} + +class FastCar extends Car {} + +class Super { + Car getCar() { + return new Car(); + } +} + +class Sub extends Super { + FastCar getCar() { + return new FastCar(); + } +} + +public class CovBaseProgram01 { + public static void main(String[] argv) { + new CovBaseProgram01().run(); + } + + public void run() { + Super instance_super = new Super(); + Sub instance_sub = new Sub(); + + Car c1 = instance_super.getCar(); // Line 26 + Car c2 = instance_sub.getCar(); // Line 27 + } +} + +// Line26: callJPs: call(Car Super.getCar()) +// Line27: callJPs: call(FastCar Sub.getCar()) call(Car Super.getCar()) + + */ + +/** + * Covariance is simply where a type overrides some inherited implementation and narrows the return type. + */ +public class CovarianceTests extends AjcTestCase { + + private boolean verbose = false; + + + /** + * call(* getCar()) should match both + */ + public void testCOV001() { + CompilationResult cR = binaryWeave("CovBaseProgram01.jar","CovAspect01.aj",0,0); + verifyOutput(cR,new String[]{ + "weaveinfo Type 'CovBaseProgram01' (CovBaseProgram01.java:26) advised by before advice from 'CovAspect01' (CovAspect01.aj:5)", + "weaveinfo Type 'CovBaseProgram01' (CovBaseProgram01.java:27) advised by before advice from 'CovAspect01' (CovAspect01.aj:5)" + }); + } + + + /** + * call(* Super.getCar()) should match both + * + * This test required a change to the compiler. When we are looking at signatures and comparing them we walk up + * the hierarchy looking for supertypes that declare the same method. The problem is that in the comparison for + * whether to methods are compatible we were including the return type - this meant 'Car getCar()' on Super was + * different to 'FastCar getCar()' on Sub - it thought they were entirely different methods. In fact the return + * type is irrelevant here, we just want to make sure the names and the parameter types are the same - so I + * added a parameterSignature to the Member class that looks like '()' where the full signature looks like + * '()LFastCar;' (which includes the return type). If the full signature comparison fails then it looks at the + * parameter signature - I did it that way to try and preserve some performance. I haven't changed the + * definition of 'signature' for a member as trimming the return type off it seems rather serious ! + * + * What might break: + * - 'matches' can now return true for things that have different return types - I guess whether this is a problem + * depends on what the caller of matches is expecting, their code will have been written before covariance was + * a possibility. All the tests pass so I'll leave it like this for now. + */ + public void testCOV002() { + CompilationResult cR = binaryWeave("CovBaseProgram01.jar","CovAspect02.aj",0,0); + verifyOutput(cR,new String[]{ + "weaveinfo Type 'CovBaseProgram01' (CovBaseProgram01.java:26) advised by before advice from 'CovAspect02' (CovAspect02.aj:5)", + "weaveinfo Type 'CovBaseProgram01' (CovBaseProgram01.java:27) advised by before advice from 'CovAspect02' (CovAspect02.aj:5)" + }); + } + + /** + * call(Car getCar()) should match both + * + * Had to implement proper covariance support here... + */ + public void testCOV003() { + CompilationResult cR = binaryWeave("CovBaseProgram01.jar","CovAspect03.aj",0,0); + + verifyOutput(cR,new String[]{ + "weaveinfo Type 'CovBaseProgram01' (CovBaseProgram01.java:26) advised by before advice from 'CovAspect03' (CovAspect03.aj:5)", + "weaveinfo Type 'CovBaseProgram01' (CovBaseProgram01.java:27) advised by before advice from 'CovAspect03' (CovAspect03.aj:5)" + }); + } + + /** + * *** Different base program, where Sub does not extend Super. + * call(Car Super.getCar()) should only match first call to getCar() + */ + public void testCOV004() { + CompilationResult cR = binaryWeave("CovBaseProgram02.jar","CovAspect04.aj",0,0); + verifyOutput(cR,new String[]{ + "weaveinfo Type 'CovBaseProgram02' (CovBaseProgram02.java:30) advised by before advice from 'CovAspect04' (CovAspect04.aj:5)" + }); + } + + /** + * *** Original base program + * call(Car Super.getCar()) should match both + */ + public void testCOV005() { + CompilationResult cR = binaryWeave("CovBaseProgram01.jar","CovAspect05.aj",0,0); + verifyOutput(cR,new String[]{ + "weaveinfo Type 'CovBaseProgram01' (CovBaseProgram01.java:26) advised by before advice from 'CovAspect05' (CovAspect05.aj:5)", + "weaveinfo Type 'CovBaseProgram01' (CovBaseProgram01.java:27) advised by before advice from 'CovAspect05' (CovAspect05.aj:5)" + }); + } + + /** + * call(Car Sub.getCar()) should not match anything + */ + public void testCOV006() { + CompilationResult cR = binaryWeave("CovBaseProgram01.jar","CovAspect06.aj",0,1); + verifyOutput(cR,new String[]{/* no expected output! */}); + assertTrue("Expected one xlint warning message for line 26, but got: "+cR.getWarningMessages(), + cR.getWarningMessages().size()==1 && ((IMessage)cR.getWarningMessages().get(0)).toString().indexOf("26")!=-1); + + } + + /** + * call(Car+ Sub.getCar()) should match 2nd call with xlint for the 1st call + */ + public void testCOV007() { + CompilationResult cR = binaryWeave("CovBaseProgram01.jar","CovAspect07.aj",0,1); + verifyOutput(cR,new String[]{ + "weaveinfo Type 'CovBaseProgram01' (CovBaseProgram01.java:27) advised by before advice from 'CovAspect07' (CovAspect07.aj:5)" + }); + assertTrue("Expected one xlint warning message for line 26, but got: "+cR.getWarningMessages(), + cR.getWarningMessages().size()==1 && ((IMessage)cR.getWarningMessages().get(0)).toString().indexOf("26")!=-1); + } + + /** + * *** aspect now contains two pointcuts and two pieces of advice + * call(FastCar getCar()) matches on 2nd call + * call(FastCar Sub.getCar()) matches on 2nd call + */ + public void testCOV008() { + CompilationResult cR = binaryWeave("CovBaseProgram01.jar","CovAspect08.aj",0,0); + verifyOutput(cR,new String[]{ + "weaveinfo Type 'CovBaseProgram01' (CovBaseProgram01.java:27) advised by before advice from 'CovAspect08' (CovAspect08.aj:11)", + "weaveinfo Type 'CovBaseProgram01' (CovBaseProgram01.java:27) advised by before advice from 'CovAspect08' (CovAspect08.aj:5)" + }); + } + + /** + * call(FastCar Super.getCar()) matches nothing + */ + public void testCOV009() { + CompilationResult cR = binaryWeave("CovBaseProgram01.jar","CovAspect09.aj",0,0); + verifyOutput(cR,new String[]{/* No matches */}); + assertTrue("Expected no warnings but got: "+cR.getWarningMessages(),cR.getWarningMessages().size()==0); + } + + /** + * call(Car+ getCar()) matches both + */ + public void testCOV010() { + CompilationResult cR = binaryWeave("CovBaseProgram01.jar","CovAspect10.aj",0,0); + verifyOutput(cR,new String[]{ + "weaveinfo Type 'CovBaseProgram01' (CovBaseProgram01.java:26) advised by before advice from 'CovAspect10' (CovAspect10.aj:5)", + "weaveinfo Type 'CovBaseProgram01' (CovBaseProgram01.java:27) advised by before advice from 'CovAspect10' (CovAspect10.aj:5)" + }); + } + + //-------------------------------------------------------------------------------- + //-------------------------------------------------------------------------------- + //-------------------------------------------------------------------------------- + + private File baseDir; + + protected void setUp() throws Exception { + super.setUp(); + baseDir = new File("../tests/java5/covariance"); + } + + private CompilationResult binaryWeave(String inpath, String insource,int expErrors,int expWarnings) { + String[] args = new String[] {"-inpath",inpath,insource,"-showWeaveInfo"}; + CompilationResult result = ajc(baseDir,args); + if (verbose || result.hasErrorMessages()) System.out.println(result); + assertTrue("Expected "+expErrors+" errors but got "+result.getErrorMessages().size()+":\n"+ + formatCollection(result.getErrorMessages()),result.getErrorMessages().size()==expErrors); + assertTrue("Expected "+expWarnings+" warnings but got "+result.getWarningMessages().size()+":\n"+ + formatCollection(result.getWarningMessages()),result.getWarningMessages().size()==expWarnings); + return result; + } + + private List getWeavingMessages(List msgs) { + List result = new ArrayList(); + for (Iterator iter = msgs.iterator(); iter.hasNext();) { + IMessage element = (IMessage) iter.next(); + if (element.getKind()==IMessage.WEAVEINFO) { + result.add(element.toString()); + } + } + return result; + } + + private void verifyOutput(CompilationResult cR,String[] expected) { + List weavingmessages = getWeavingMessages(cR.getInfoMessages()); + dump(weavingmessages); + for (int i = 0; i < expected.length; i++) { + boolean found = weavingmessages.contains(expected[i]); + if (found) { + weavingmessages.remove(expected[i]); + } else { + System.err.println(dump(getWeavingMessages(cR.getInfoMessages()))); + fail("Expected message not found.\nExpected:\n"+expected[i]+"\nObtained:\n"+dump(getWeavingMessages(cR.getInfoMessages()))); + } + } + if (weavingmessages.size()!=0) { + fail("Unexpected messages obtained from program:\n"+dump(weavingmessages)); + } + } + + private String formatCollection(Collection s) { + StringBuffer sb = new StringBuffer(); + for (Iterator iter = s.iterator(); iter.hasNext();) { + Object element = (Object) iter.next(); + sb.append(element).append("\n"); + } + return sb.toString(); + } + + private static Set split(String input) { + Set l = new HashSet(); + int idx = 0; + while (input.indexOf("]",idx)!=-1) { + int nextbreak = input.indexOf("]",idx); + String s = input.substring(idx,nextbreak+1); + + l.add(s); + idx = input.indexOf("[",nextbreak+1); + if (idx==-1) break; + } + return l; + } + + private void copyFile(String fromName) { + copyFile(fromName,fromName); + } + + private void copyFile(String from,String to) { + try { + org.aspectj.util.FileUtil.copyFile(new File(baseDir + File.separator + from), + new File(ajc.getSandboxDirectory(),to)); + } catch (IOException ioe) { + ioe.printStackTrace(); + } + } + + + private String dump(List l) { + StringBuffer sb = new StringBuffer(); + int i =0; + sb.append("--- Weaving Messages ---\n"); + for (Iterator iter = l.iterator(); iter.hasNext();) { + sb.append(i+") "+iter.next()+"\n"); + } + sb.append("------------------------\n"); + return sb.toString(); + } +} diff --git a/weaver/src/org/aspectj/weaver/Member.java b/weaver/src/org/aspectj/weaver/Member.java index d30a60f8e..3f7e9205c 100644 --- a/weaver/src/org/aspectj/weaver/Member.java +++ b/weaver/src/org/aspectj/weaver/Member.java @@ -33,6 +33,7 @@ public class Member implements Comparable { private final String name; private final TypeX[] parameterTypes; private final String signature; + private String paramSignature; public Member( Kind kind, @@ -381,8 +382,29 @@ public class Member implements Comparable { public TypeX getType() { return returnType; } public String getName() { return name; } public TypeX[] getParameterTypes() { return parameterTypes; } + /** + * Return full signature, including return type, e.g. "()LFastCar;" for a signature without the return type, + * use getParameterSignature() - it is importnant to choose the right one in the face of covariance. + */ public String getSignature() { return signature; } public int getArity() { return parameterTypes.length; } + + /** + * Return signature without return type, e.g. "()" for a signature *with* the return type, + * use getSignature() - it is important to choose the right one in the face of covariance. + */ + public String getParameterSignature() { + if (paramSignature != null) return paramSignature; + StringBuffer sb = new StringBuffer(); + sb.append("("); + for (int i = 0; i < parameterTypes.length; i++) { + TypeX tx = parameterTypes[i]; + sb.append(tx.getSignature()); + } + sb.append(")"); + paramSignature = sb.toString(); + return paramSignature; + } public boolean isCompatibleWith(Member am) { if (kind != METHOD || am.getKind() != METHOD) return true; diff --git a/weaver/src/org/aspectj/weaver/ResolvedTypeX.java b/weaver/src/org/aspectj/weaver/ResolvedTypeX.java index 45d24e79e..bb3dda37e 100644 --- a/weaver/src/org/aspectj/weaver/ResolvedTypeX.java +++ b/weaver/src/org/aspectj/weaver/ResolvedTypeX.java @@ -215,12 +215,25 @@ public abstract class ResolvedTypeX extends TypeX { } /** - * described in JVM spec 2ed 5.4.3.3 + * described in JVM spec 2ed 5.4.3.3. + * Doesnt check ITDs. */ public ResolvedMember lookupMethod(Member m) { return lookupMember(m, getMethods()); } + public ResolvedMember lookupMethodInITDs(Member m) { + if (interTypeMungers != null) { + for (Iterator i = interTypeMungers.iterator(); i.hasNext();) { + ConcreteTypeMunger tm = (ConcreteTypeMunger) i.next(); + if (matches(tm.getSignature(), m)) { + return tm.getSignature(); + } + } + } + return null; + } + /** return null if not found */ private ResolvedMember lookupMember(Member m, Iterator i) { while (i.hasNext()) { @@ -244,7 +257,19 @@ public abstract class ResolvedTypeX extends TypeX { public static boolean matches(Member m1, Member m2) { if (m1 == null) return m2 == null; if (m2 == null) return false; - return m1.getName().equals(m2.getName()) && m1.getSignature().equals(m2.getSignature()); + + // Check the names + boolean equalNames = m1.getName().equals(m2.getName()); + if (!equalNames) return false; + + // Check the signatures + boolean equalSignatures = m1.getSignature().equals(m2.getSignature()); + if (equalSignatures) return true; + + // If they aren't the same, we need to allow for covariance ... where one sig might be ()LCar; and + // the subsig might be ()LFastCar; - where FastCar is a subclass of Car + boolean equalParamSignatures = m1.getParameterSignature().equals(m2.getParameterSignature()); + return equalParamSignatures; } @@ -908,7 +933,9 @@ public abstract class ResolvedTypeX extends TypeX { } - /** return null if not found */ + /** + * Look up a member, takes into account any ITDs on this type. + * return null if not found */ public ResolvedMember lookupMemberNoSupers(Member member) { ResolvedMember ret; if (member.getKind() == Member.FIELD) { diff --git a/weaver/src/org/aspectj/weaver/patterns/KindedPointcut.java b/weaver/src/org/aspectj/weaver/patterns/KindedPointcut.java index 662a21afe..65da3efac 100644 --- a/weaver/src/org/aspectj/weaver/patterns/KindedPointcut.java +++ b/weaver/src/org/aspectj/weaver/patterns/KindedPointcut.java @@ -122,6 +122,13 @@ public class KindedPointcut extends Pointcut { return; } + if (!signature.getReturnType().matchesStatically(shadow.getSignature().getReturnType().resolve(world))) { + // Covariance issue... + // The reason we didn't match is that the type pattern for the pointcut (Car) doesn't match the + // return type for the specific declaration at the shadow. (FastCar Sub.getCar()) + // XXX Put out another XLINT in this case? + return; + } // PR60015 - Don't report the warning if the declaring type is object and 'this' is an interface if (exactDeclaringType.isInterface(world) && shadowDeclaringType.equals(world.resolve("java.lang.Object"))) { return; diff --git a/weaver/src/org/aspectj/weaver/patterns/SignaturePattern.java b/weaver/src/org/aspectj/weaver/patterns/SignaturePattern.java index f9a7373fc..edaa45d8a 100644 --- a/weaver/src/org/aspectj/weaver/patterns/SignaturePattern.java +++ b/weaver/src/org/aspectj/weaver/patterns/SignaturePattern.java @@ -153,13 +153,20 @@ public class SignaturePattern extends PatternNode { //System.out.println(" ret: " + ret); return ret; } else if (kind == Member.METHOD) { - if (!returnType.matchesStatically(sig.getReturnType().resolve(world))) return false; + // Change all this in the face of covariance... + + // Check the name if (!name.matches(sig.getName())) return false; + + // Check the parameters if (!parameterTypes.matches(world.resolve(sig.getParameterTypes()), TypePattern.STATIC).alwaysTrue()) { return false; } + + // Check the throws pattern if (!throwsPattern.matches(sig.getExceptions(), world)) return false; - return declaringTypeMatch(member.getDeclaringType(), member, world); + + return declaringTypeMatchAllowingForCovariance(member,world,returnType,sig.getReturnType().resolve(world)); } else if (kind == Member.CONSTRUCTOR) { if (!parameterTypes.matches(world.resolve(sig.getParameterTypes()), TypePattern.STATIC).alwaysTrue()) { return false; @@ -171,7 +178,43 @@ public class SignaturePattern extends PatternNode { return false; } - + + public boolean declaringTypeMatchAllowingForCovariance(Member member,World world,TypePattern returnTypePattern,ResolvedTypeX sigReturn) { + TypeX onTypeUnresolved = member.getDeclaringType(); + + ResolvedTypeX onType = onTypeUnresolved.resolve(world); + + // fastmatch + if (declaringType.matchesStatically(onType) && returnTypePattern.matchesStatically(sigReturn)) + return true; + + Collection declaringTypes = member.getDeclaringTypes(world); + + boolean checkReturnType = true; + // XXX Possible enhancement? Doesn't seem to speed things up + // if (returnTypePattern.isStar()) { + // if (returnTypePattern instanceof WildTypePattern) { + // if (((WildTypePattern)returnTypePattern).getDimensions()==0) checkReturnType = false; + // } + // } + + // Sometimes that list includes types that don't explicitly declare the member we are after - + // they are on the list because their supertype is on the list, that's why we use + // lookupMethod rather than lookupMemberNoSupers() + for (Iterator i = declaringTypes.iterator(); i.hasNext(); ) { + ResolvedTypeX type = (ResolvedTypeX)i.next(); + if (declaringType.matchesStatically(type)) { + if (!checkReturnType) return true; + ResolvedMember rm = type.lookupMethod(member); + if (rm==null) rm = type.lookupMethodInITDs(member); // It must be in here, or we have *real* problems + TypeX returnTypeX = rm.getReturnType(); + ResolvedTypeX returnType = returnTypeX.resolve(world); + if (returnTypePattern.matchesStatically(returnType)) return true; + } + } + return false; + } + // for dynamic join point matching public boolean matches(JoinPoint.StaticPart jpsp) { Signature sig = jpsp.getSignature(); @@ -207,7 +250,7 @@ public class SignaturePattern extends PatternNode { return false; } if (!throwsPattern.matches(exceptionTypes)) return false; - return declaringTypeMatch(sig); + return declaringTypeMatch(sig); // XXXAJ5 - Need to make this a covariant aware version for dynamic JP matching to work } else if (kind == Member.CONSTRUCTOR) { ConstructorSignature csig = (ConstructorSignature)sig; Class[] params = csig.getParameterTypes(); @@ -223,6 +266,7 @@ public class SignaturePattern extends PatternNode { return false; } +// For methods, the above covariant aware version (declaringTypeMatchAllowingForCovariance) is used - this version is still here for fields private boolean declaringTypeMatch(TypeX onTypeUnresolved, Member member, World world) { ResolvedTypeX onType = onTypeUnresolved.resolve(world); diff --git a/weaver/src/org/aspectj/weaver/patterns/WildTypePattern.java b/weaver/src/org/aspectj/weaver/patterns/WildTypePattern.java index d87ade6b1..fa2e644f9 100644 --- a/weaver/src/org/aspectj/weaver/patterns/WildTypePattern.java +++ b/weaver/src/org/aspectj/weaver/patterns/WildTypePattern.java @@ -86,6 +86,14 @@ public class WildTypePattern extends TypePattern { return matchesExactlyByName(targetTypeName); } + /** + * Used in conjunction with checks on 'isStar()' to tell you if this pattern represents '*' or '*[]' which are + * different ! + */ + public int getDimensions() { + return dim; + } + /** * @param targetTypeName * @return