Browse Source

Support for @RequiredTypes and abstract aspects

tags/V1_8_3
Andy Clement 9 years ago
parent
commit
0df90e175b

+ 114
- 0
docs/dist/doc/README-183.html View File

@@ -0,0 +1,114 @@
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 Final//EN">
<html> <head>
<title>AspectJ 1.8.3 Readme</title>
<style type="text/css">
<!--
P { margin-left: 20px; }
PRE { margin-left: 20px; }
LI { margin-left: 20px; }
H4 { margin-left: 20px; }
H3 { margin-left: 10px; }
-->
</style>
</head>

<body>
<div align="right"><small>
&copy; Copyright 2014 Contributors.
All rights reserved.
</small></div>

<h1>AspectJ 1.8.3 Readme</h1>

<p>The full list of resolved issues in 1.8.3 is available
<a href="https://bugs.eclipse.org/bugs/buglist.cgi?query_format=advanced;bug_status=RESOLVED;bug_status=VERIFIED;bug_status=CLOSED;product=AspectJ;target_milestone=1.8.3;">here</a></h2>.</p>

<ul>
<li>1.8.3 available 22-Oct-2014
</ul>

<h2>Notable changes</h2>

<h3>Conditional aspect activation with <tt>@RequiredTypes</tt> - <a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=436653">Issue 436653</a></h3>

<p>AspectJ is sometimes used to create aspect libraries. These libraries contain a number of aspects often covering
a variety of domains. The library might typically be available as a jar and contains a single aop.xml file that
names all the aspects. The library is then consumed by some application.
However, the application may not need to use all those aspects
but because they are listed in the aop.xml they will be 'active'. Now the pointcuts in those unused aspects
may not match anything in the application and could be considered harmless but the pointcuts and the aspects
themselves may have references to types in other libraries that the application does not have around. This can lead
to unhelpful <tt>"can't find type"</tt> messages and currently requires the user to add unnecessary entries to their
build dependencies just to keep the unused aspects happy.
</p>
<p>With AspectJ 1.8.3 it is now possible to express a constraint on an aspect. The <tt>@RequiredTypes</tt>
annotation specifies one or more fully qualified types that must be discoverable on the classpath in
order for the aspect to activate. Using this there is no need to add those extraneous dependencies to
an applications build classpath.
</p>
<p>Example:</p>
<pre><code>import org.aspectj.lang.annotation.*;

@RequiredTypes("com.foo.Bar")
public aspect Foo {
before(): execution(@com.foo.Bar * *(..)) {}
}
</code></pre>

<p>
If the above aspect is listed in an aop.xml for loadtime weaving or passed on the aspectpath for
compile time weaving, if the type <tt>'com.foo.Bar'</tt> is not accessible on the classpath then the
aspect will be turned off and the pointcut will have no effect. There will be no attempt made to
match it and so no unhelpful <tt>"can't find type"</tt> messages.
</p>

<h3>cflow and the pre-initialization joinpoint changes due to Java 7 verifier modifications - <a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=443477">Issue 443477</a></h3>

<p>There has been a change in the Java7 verifier in a recent patch release of Java7 (update 67) that causes
a verify error for usage of a particular AspectJ construct. The problem occurs if you are using
cflow and it hits the preinitialization join point. The pattern of code generated in that case causes
the verifyerror. In this release of AspectJ we have taken the 'quick' approach to solving this, namely
to avoid advising preinitialization with the cflow construct. This problem appears to come up
when the aspect is non-optimal anyway and hitting preinitialization was never really intended by the
pointcut writer. For example:

<pre><code>execution(* foo(..)) && cflow(within(Bar))</code></pre>

<p>The use of cflow and within there will actually hit *a lot* of joinpoints, many of which the user probably didn't mean to.
It feels like we actually need a warning to indicate the pointcut is probably suboptimal. What the user probably
meant was something more like this:</p>

<pre><code>execution(* foo(..)) && cflow(execution(* Bar.*(..))</code></pre>
<p>or</p>
<pre><code>execution(* foo(..)) && cflow(within(Bar) && execution(* *(..)))</code></pre>

<p>
But even if they did want the less optimal form of cflow there still seems little use in applying it to
pre-initialization - that is your cue to raise an AspectJ bug with a realistic use case inside that proves this
an invalid assumption :)</p>

<h3>around advice and lambdas - <a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=445395">Issue 445395</a></h3>

<p>For optimal performance, where possible, AspectJ tries to inline around advice when it applies
at a joinpoint. There are few characteristics of a joinpoint match that can prevent this but we
do try to inline where we can (the inlining can be manually turned off via <tt>-XnoInline</tt>).</p>

<p>Inlining of around advice basically means copying the advice instructions into the target class. This causes
a problem when the advice uses lambdas. Lambda usage is currently implemented in java compilers by generating
invokedynamic bytecode instructions that reference bootstrap methods created in the class and a helper method
generated in the class containing the lambda code. When the invokedynamic is encountered at runtime, some magic
happens and the bootstrap method is used to generate a class on the fly that calls the particular lambda method.
All this 'extra stuff' breaks the basic inlining algorithm that simply copies the advice bytecode into the target.
Effectively the inlining process needs to become much more sophisticated and copy the bootstrap methods and
the lambda helper methods, avoiding clashes with existing bootstrap/helpers in the target.</p>

<p>
Prior to AspectJ 1.8.3 when the inlining failed you would get a horrible class cast exception that mentions
constant pool entries (because the bootstrap method hadn't been copied over to the target). Temporarily in
1.8.3 we are turning off inlining of around advice containing lambdas, which will at least avoid the failure,
with the longer term goal of improving the inlining process to do all the necessary extra work.
</p>

<!-- ============================== -->
</body>
</html>

+ 1
- 0
docs/dist/doc/index.html View File

@@ -138,6 +138,7 @@
<tr> <td>README's
</td>
<td>Changes and porting guide for AspectJ
<a href="README-183.html">1.8.2</a>,
<a href="README-182.html">1.8.2</a>,
<a href="README-181.html">1.8.1</a>,
<a href="README-180.html">1.8.0</a>,

+ 5
- 0
org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseSourceType.java View File

@@ -818,6 +818,11 @@ public class EclipseSourceType extends AbstractReferenceTypeDelegate {
AnnotationValue array = new ArrayAnnotationValue(arrayValues);
AnnotationNameValuePair anvp = new AnnotationNameValuePair(new String(mvp.name), array);
annotationAJ.addNameValuePair(anvp);
} else if (mvp.value instanceof Literal) {
AnnotationValue av = generateElementValue(mvp.value,
((Literal) mvp.value).resolvedType);
AnnotationNameValuePair anvp = new AnnotationNameValuePair(new String(mvp.name), av);
annotationAJ.addNameValuePair(anvp);
} else {
MethodBinding methodBinding = mvp.binding;
if (methodBinding == null) {

+ 39
- 11
org.aspectj.matcher/src/org/aspectj/weaver/CrosscuttingMembersSet.java View File

@@ -23,14 +23,14 @@ import java.util.List;
import java.util.Map;
import java.util.Set;

import org.aspectj.bridge.IMessage;
import org.aspectj.bridge.MessageUtil;
import org.aspectj.weaver.patterns.Declare;
import org.aspectj.weaver.patterns.DeclareAnnotation;
import org.aspectj.weaver.patterns.DeclareParents;
import org.aspectj.weaver.patterns.DeclareSoft;
import org.aspectj.weaver.patterns.DeclareTypeErrorOrWarning;
import org.aspectj.weaver.patterns.IVerificationRequired;
import org.aspectj.weaver.tools.Trace;
import org.aspectj.weaver.tools.TraceFactory;

/**
* This holds on to all CrosscuttingMembers for a world. It handles management of change.
@@ -40,15 +40,13 @@ import org.aspectj.weaver.tools.TraceFactory;
*/
public class CrosscuttingMembersSet {

private static Trace trace = TraceFactory.getTraceFactory().getTrace(CrosscuttingMembersSet.class);

private transient World world;

// FIXME AV - ? we may need a sequencedHashMap there to ensure source based precedence for @AJ advice
private final Map /* ResolvedType (the aspect) > CrosscuttingMembers */<ResolvedType, CrosscuttingMembers> members = new HashMap<ResolvedType, CrosscuttingMembers>();
private final Map<ResolvedType, CrosscuttingMembers> members = new HashMap<ResolvedType, CrosscuttingMembers>();

// List of things to be verified once the type system is 'complete'
private transient List /* IVerificationRequired */<IVerificationRequired> verificationList = null;
private transient List<IVerificationRequired> verificationList = null;

private List<ShadowMunger> shadowMungers = null;
private List<ConcreteTypeMunger> typeMungers = null;
@@ -69,17 +67,46 @@ public class CrosscuttingMembersSet {
public boolean addOrReplaceAspect(ResolvedType aspectType) {
return addOrReplaceAspect(aspectType, true);
}
/**
* Check if any parent aspects of the supplied aspect have unresolved dependencies (and so
* should cause this aspect to be turned off).
* @param aspectType the aspect whose parents should be checked
* @return true if this aspect should be excluded because of a parents' missing dependencies
*/
private boolean excludeDueToParentAspectHavingUnresolvedDependency(ResolvedType aspectType) {
ResolvedType parent = aspectType.getSuperclass();
boolean excludeDueToParent = false;
while (parent != null) {
if (parent.isAspect() && parent.isAbstract() && world.hasUnsatisfiedDependency(parent)) {
if (!world.getMessageHandler().isIgnoring(IMessage.INFO)) {
world.getMessageHandler().handleMessage(
MessageUtil.info("deactivating aspect '" + aspectType.getName() + "' as the parent aspect '"+parent.getName()+
"' has unsatisfied dependencies"));
}
excludeDueToParent = true;
}
parent = parent.getSuperclass();
}
return excludeDueToParent;
}

/**
* @return whether or not that was a change to the global signature XXX for efficiency we will need a richer representation than
* this
*/
public boolean addOrReplaceAspect(ResolvedType aspectType, boolean inWeavingPhase) {

if (!world.isAspectIncluded(aspectType) || world.hasUnsatisfiedDependency(aspectType)) {
if (!world.isAspectIncluded(aspectType)) {
return false;
}

if (world.hasUnsatisfiedDependency(aspectType)) {
return false;
}
// Abstract super aspects might have unsatisfied dependencies
if (excludeDueToParentAspectHavingUnresolvedDependency(aspectType)) {
return false;
}
boolean change = false;
CrosscuttingMembers xcut = members.get(aspectType);
if (xcut == null) {
@@ -108,14 +135,15 @@ public class CrosscuttingMembersSet {

return change;
}
private boolean addOrReplaceDescendantsOf(ResolvedType aspectType, boolean inWeavePhase) {
// System.err.println("Looking at descendants of "+aspectType.getName());
Set<ResolvedType> knownAspects = members.keySet();
Set<ResolvedType> toBeReplaced = new HashSet<ResolvedType>();
for (Iterator<ResolvedType> it = knownAspects.iterator(); it.hasNext();) {
ResolvedType candidateDescendant = it.next();
if ((candidateDescendant != aspectType) && (aspectType.isAssignableFrom(candidateDescendant))) {
// allowMissing = true - if something is missing, it really probably is not a descendant
if ((candidateDescendant != aspectType) && (aspectType.isAssignableFrom(candidateDescendant, true))) {
toBeReplaced.add(candidateDescendant);
}
}

+ 1
- 1
runtime/testsrc/org/aspectj/runtime/reflect/RuntimePerformanceTest.java View File

@@ -78,7 +78,7 @@ public class RuntimePerformanceTest extends TestCase {

long ratio = (EXPECTED_RATIO*noCache/cache);
System.out.println("ratio=" + ratio);
assertTrue("Using cache should be " + EXPECTED_RATIO + " times faster: " + ratio,(ratio > EXPECTED_RATIO));
assertTrue("Using cache should be " + EXPECTED_RATIO + " times faster: " + ratio,(ratio >= EXPECTED_RATIO));
}
private long invokeSignatureToString (Signature sig, long iterations) {

+ 1
- 0
tests/bugs183/436653/abstract/A.java View File

@@ -0,0 +1 @@
public class A {}

+ 5
- 0
tests/bugs183/436653/abstract/AA.java View File

@@ -0,0 +1,5 @@
import org.aspectj.lang.annotation.*;

@RequiredTypes("A")
abstract aspect AA {
}

+ 8
- 0
tests/bugs183/436653/abstract/Code.java View File

@@ -0,0 +1,8 @@
public class Code {
public static void main(String []argv) {
new Code().m();
}
public void m() {
System.out.println("Code.m()");
}
}

+ 8
- 0
tests/bugs183/436653/abstract/X.java View File

@@ -0,0 +1,8 @@
import org.aspectj.lang.annotation.*;

//@RequiredTypes("A")
aspect X extends AA {
@SuppressAjWarnings("adviceDidNotMatch")
before(): execution(* *(..)) { System.out.println("X.before"); }
}


+ 8
- 0
tests/src/org/aspectj/systemtest/ajc183/Ajc183Tests.java View File

@@ -25,6 +25,14 @@ public class Ajc183Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
runTest("super calls");
}

public void testSuppressTypeNotFoundAbstract_436653_2() {
runTest("suppress type not found - abstract 2");
}
public void testSuppressTypeNotFoundAbstract_436653_1() {
runTest("suppress type not found - abstract 1");
}

public void testSuppressTypeNotFound_436653() {
runTest("suppress type not found");
}

+ 34
- 0
tests/src/org/aspectj/systemtest/ajc183/ajc183.xml View File

@@ -10,6 +10,40 @@
</stdout>
</run>
</ajc-test>

<ajc-test dir="bugs183/436653/abstract" title="suppress type not found - abstract 2">
<compile options="-1.8 -Xlint:ignore" files="A.java" outjar="codeA.jar"/>
<compile options="-1.8" files="X.java AA.java" classpath="codeA.jar" outjar="aspects.jar"/>
<compile options="-1.8" files="Code.java" aspectpath="aspects.jar"/>
<run class="Code">
<stdout>
<!--
These don't come out because AA is missing required type A in the last compile step
<line text="X.before"/>
<line text="X.before"/>
-->
<line text="Code.m()"/>
</stdout>
</run>
</ajc-test>
<ajc-test dir="bugs183/436653/abstract" title="suppress type not found - abstract 1">
<compile options="-1.8 -Xlint:ignore" files="A.java" outjar="codeA.jar"/>
<compile options="-1.8" files="AA.java X.java" classpath="codeA.jar" outjar="aspects.jar"/>
<compile options="-1.8" files="Code.java" aspectpath="aspects.jar"/>
<run class="Code">
<stdout>
<!--
These don't come out because AA is missing required type A in the last compile step.
Difference between this test and the previous one is that this is a different order
of aspects (addOrReplaceAspects called with the abstract aspect first here).
<line text="X.before"/>
<line text="X.before"/>
-->
<line text="Code.m()"/>
</stdout>
</run>
</ajc-test>
<ajc-test dir="bugs183/436653" title="suppress type not found 4">
<compile options="-1.8 -Xlint:ignore" files="X.java" outjar="aspects.jar"/>

+ 4
- 4
weaver/src/org/aspectj/weaver/ltw/LTWWorld.java View File

@@ -176,8 +176,8 @@ public class LTWWorld extends BcelWorld implements IReflectionWorld {
if (concurrentMapClass != null) {
try {
return (Map) concurrentMapClass.newInstance();
} catch (InstantiationException _) {
} catch (IllegalAccessException _) {
} catch (InstantiationException ie) {
} catch (IllegalAccessException iae) {
}
// fall through if exceptions
}
@@ -191,9 +191,9 @@ public class LTWWorld extends BcelWorld implements IReflectionWorld {
for (int i = 0; i < betterChoices.length; i++) {
try {
return Class.forName(betterChoices[i]);
} catch (ClassNotFoundException _) {
} catch (ClassNotFoundException cnfe) {
// try the next one
} catch (SecurityException _) {
} catch (SecurityException se) {
// you get one of these if you dare to try to load an undefined class in a
// package starting with java like java.util.concurrent
}

Loading…
Cancel
Save