Bug 125981 - Declare Soft not Working on Anonymous Class
Summary: Declare Soft not Working on Anonymous Class
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: 1.5.0   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 1.5.3   Edit
Assignee: aspectj inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-02-01 02:01 EST by Ron Bodkin CLA
Modified: 2006-11-09 08:37 EST (History)
0 users

See Also:


Attachments
failing testcase and proposed fix (4.74 KB, application/zip)
2006-09-22 06:27 EDT, Helen Beeken CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ron Bodkin CLA 2006-02-01 02:01:07 EST
This code should compile with AspectJ but AspectJ 1.5.0 doesn't soften the exception properly. Here's the output I see:

C:\devel\scratch>ajc SampleTest.aj
C:\devel\scratch\SampleTest.aj:10 [error] Unhandled exception type IOException
str.read();//comment out to see that it matches declare warning
^^^^

1 error

C:\devel\scratch>ajc -version
AspectJ Compiler 1.5.0 built on Tuesday Dec 20, 2005 at 12:05:54 GMT

public class SampleTest {
    public interface ByteReadingStrategy {
        void readBytes(java.io.InputStream str);
    }

    public ByteReadingStrategy byteReadingStrategy;

    private final ByteReadingStrategy offsetBuf = new ByteReadingStrategy() {
        public void readBytes(java.io.InputStream str) {
	    str.read();
        }
    };

   private class NamedByteReadingStrategy {
        public void readBytes(java.io.InputStream str) {
	    str.read();
        }
    };

    public void foo(){}
}

aspect Soften {
    pointcut softenedTests() : 
	within(SampleTest+) && execution(* *(..)) && !execution(* *(..) throws Exception+);

    declare soft: Exception+: softenedTests();
        
    declare warning: softenedTests(): "softened test";
}
Comment 1 Helen Beeken CLA 2006-02-14 06:45:31 EST
This bug only occurs if the anonymous class is associated with a field declaration i.e.

private final ByteReadingStrategy offsetBuf = new ByteReadingStrategy() {
        public void readBytes(java.io.InputStream str) {
            str.read();
        }
    };

If it's declared within a method then everythings ok. Also, as a note, declare warnings, advice etc. match the above.
Comment 2 Helen Beeken CLA 2006-02-14 11:45:16 EST
Looking further.......if the pointcut was changed from:

pointcut softenedTests() :  within(SampleTest+) && execution(* *(..)) && !execution(* *(..) throws Exception+);

to:

pointcut softenedTests() : (within(SampleTest+) && execution(* *(..)) && !execution(* *(..) throws Exception+)) || call(* read(..)) ;

then everything works as expected. The failing scenario is when the method call (which results in the unhandled exception) is within a method execution which is declared in an anonymous class whose result is assigned to a field. When we come to look at the exceptions in AjProblemReporter.unhandledException(..), in the case of 

   public void someMethod() {
    	new ByteReadingStrategy() {
    		public void readBytes(java.io.InputStream str) {
    			str.read();
    		}
    	};
    }

(which doesn't have the unhandled exception) the shadow is method-call(int java.io.InputStream.read()) and the enclosing shadow is method-execution(void SampleTest.someMethod()), which matches declare soft. However with

private final ByteReadingStrategy offsetBuf = new ByteReadingStrategy() {
        public void readBytes(java.io.InputStream str) {
            str.read();
        }
 };

the shadow is method-call(int java.io.InputStream.read()) and the enclosing shadow is staticinitialization(void sampleTest.<clinit>()). This doesn't match and the resultant unhandled exception is thrown.
Comment 3 Ron Bodkin CLA 2006-02-14 11:58:26 EST
Interesting. Surely the enclosing shadow in both of these cases (for comment #2) should be method-execution(public void readBytes(java.io.InputStream str)). The first one works because of an accident of how the pointcut was written (i.e., it sounds like that's another bug that can be illustrated by using a narrower pointcut)

The enclosing shadow has to be the immediately enclosing shadow and not an outer shadow (containing method/initialization).
Comment 4 Andrew Clement CLA 2006-02-15 03:12:12 EST
Based on Helens investigations, I gave the compiler a run against the code originally reported in the bug and I see in the failing case:

shadow: method-call(int java.io.InputStream.read())
enclosingshadow: staticinitialization(void SampleTest.<clinit>())

and in the working case

shadow: method-call(int java.io.InputStream.read())
enclosingshadow: method-execution(void SampleTest$NamedByteReadingStrategy.readBytes(java.io.InputStream))

This could be a quirk of the JDT compiler - their problem reporting logic isn't as dependent on having exactly the right problem reporter instance as ours is in this case (where we have to work out whether or not to report it as an actual problem).  A change to the end of FlowContext.checkExceptionHandlers, from:

scope.problemReporter().unhandledException(exception,location);

to

ProblemReporter problemReporter = 
   scope.referenceCompilationUnit().problemReporter;
problemReporter.referenceContext = scope.referenceContext();
problemReporter.unhandledException(exception, location);

appears to fix the problem, giving these shadows:

method-call(int java.io.InputStream.read())
method-execution(void SampleTest$1.readBytes(java.io.InputStream))

method-call(int java.io.InputStream.read())
method-execution(void SampleTest$NamedByteReadingStrategy.readBytes(java.io.InputStream))

The problem is in the problemReporter() call which moves outwards from the scope looking for the outermost methodscope, in the case of the field initialization we end up at a peculiar methodscope whose reference context is a typedeclaration rather than a method declaration (and thats the one that gives us the staticinit() shadow).  We could investigate why the referencecontext is set like that for the method scope ... or we could just stick in the fix above which fixes this problem and passes all the other tests ;)
Comment 5 Ron Bodkin CLA 2006-02-15 13:54:11 EST
It does sound like a subtle JDT bug that would be worth reporting (they'd probably view it as an enhancement request), with a good workaround for AspectJ that is important for its semantics. Thanks Andy.
Comment 6 Wes Isberg CLA 2006-05-10 11:56:07 EDT
Andy reports problem fixed for AspectJ...
stalebug
Comment 7 Helen Beeken CLA 2006-09-22 06:18:15 EDT
The provided testcase is still failing and I don't believe the proposed fix made it into the AJ codebase. Am therefore reopening this bug.
Comment 8 Helen Beeken CLA 2006-09-22 06:27:20 EDT
Created attachment 50691 [details]
failing testcase and proposed fix

The attached zip contains the following:

* pr125981-tests.txt: apply this patch to the tests project. This is the supplied failing testcase written to fit in with the AJ testsuite.

* FlowContext.java: contains the changes proposed in comment #4 marked with "AspectJ Extension". Replace the one in org.aspectj.org.eclipse.jdt.internal.compiler.flow with this one.
Comment 9 Helen Beeken CLA 2006-09-25 03:40:20 EDT
Fix has been applied and is in latest AJ dev build (Build.824 - 09/22/2006 17:59:43)
Comment 10 Andrew Clement CLA 2006-09-25 03:54:08 EDT
fix available
Comment 11 Helen Beeken CLA 2006-11-09 08:37:24 EST
iplog