Bug 111667

Summary:

Produce a compile warning when default advice precedence is applied

Product:

[Tools] AspectJ

Reporter:

Ron DiFrango <difranr>

Component:

Compiler

Assignee:

Andrew Clement <aclement>

Status:

RESOLVED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

difranr

Version:

unspecified

Target Milestone:

1.5.0RC1

Hardware:

OS:

Windows XP

Whiteboard:

Attachments:

Description	Flags
The rewriter and testcase	none

Description Ron DiFrango

2005-10-05 14:34:46 EDT

I propose that the compiler spit out a warning anytime that it has to apply 
the default advice precedence.  Also, it should spit out the recommendation 
that default ordering is not guaranteed from release to release of the 
compiler.

You can see the thread on aspectj-dev titled "change in runtime execution 
order" and the one on aspectj-users titled "AJDT 1.3 and aspectj" for the 
reasoning behind why this is a good thing.

Comment 1 Andrew Clement

2005-10-06 05:40:18 EDT

I'm considering adding a lint warning for this which will identify when multiple
pieces of advice are hitting a join point.

My problem is whether this should be on by default.  I don't think it should
since it will be massively annoying - but how would a user know to turn it on?

hmmm...

if you want to know the order of precedence that occurred, I really think you
can get that from -showWeaveInfo - for any join point you should see the
messages in a particular order indicating the order in which the advice applied.
 its possibly not ideal but does it help in your case?

Comment 2 Ron DiFrango

2005-10-06 10:02:20 EDT

Andy, I personally am in favor of having it on all the time as I view it as 
standard compiler type warning that is telling you that are doing something 
that may/may not be the right way to go.  I think of it in a similiar fashion 
to the C/C++ cast warnings I used to get all the time.  

Now, I can understand why having it on all of the time might be annoying to 
some.  So I guess the -lint or -showWeaverInfo options are good approaches, 
but I still have the same reservations that you express.  I am not sure I 
would turn them on and this may come back to bite me.

Now of course, having been burned by this I will definitely turn it.  Shall we 
propose this to the community for responses and discussions?

Comment 3 Adrian Colyer

2005-10-28 07:13:40 EDT

andy says this will take him 5 seconds ;)

Comment 4 Andrew Clement

2005-11-28 04:45:36 EST

Created attachment 30697 [details]
The rewriter and testcase

Comment 5 Andrew Clement

2005-11-28 05:50:18 EST

Damn - that was attached to the wrong bug, ignore it.

Comment 6 Andrew Clement

2005-11-30 11:35:27 EST

err.. didnt quite take me 5 seconds.  But I now have a system that reports information like this:

Unexpected warning messages:
	warning at p1.move(1, 2);
^^^^^^^^^^^^^^
C:\temp\ajcSandbox\ajcTest1381.tmp\Driver.java:16:0::0 at this shadow method-call(void Pos.move(int, int)) no precedence is specified between advice applying from aspect Foo and aspect Bar [Xlint:unorderedAdviceAtShadow]

	warning at Tester.checkEqual(p1.getX(), 1, "p1.x");
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
C:\temp\ajcSandbox\ajcTest1381.tmp\Driver.java:17:0::0 at this shadow method-call(int Pos.getX()) no precedence is specified between advice applying from aspect Foo and aspect Bar [Xlint:unorderedAdviceAtShadow]

Seems to work ok in my minimal testing.  If I set its default level to 'warning' for the whole test suite it gives me 33 failures, all seem to be reasonable occurrences of this message.  I will set its default to 'ignore' though for what I check in as calculating what goes in the message is *not* cheap.

The xlint is called 'unorderedAdviceAtShadow'

just waiting on a test run to complete before checking it in...

Comment 7 Andrew Clement

2005-11-30 14:03:37 EST

feature available in latest aspectj - will be in AJDT in a few days.