Bug 500035 - ProceedingJoinPoint changes order of input parameters
Summary: ProceedingJoinPoint changes order of input parameters
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: 1.8.9   Edit
Hardware: PC Windows 7
: P3 major (vote)
Target Milestone: 1.8.10   Edit
Assignee: aspectj inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-08-21 04:10 EDT by Sergey Kadaner CLA
Modified: 2016-11-17 19:45 EST (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sergey Kadaner CLA 2016-08-21 04:10:10 EDT
The following example demonstrates the problem:

    @Around(value = "args(regex, replacement) && target(targetObject) " +
            "&& call(public String String.replaceFirst(String, String)) ",
            argNames = "proceedingJoinPoint,targetObject,regex,replacement,thisJoinPoint")
    public String replaceFirstAspect(ProceedingJoinPoint proceedingJoinPoint, String targetObject, String regex, String replacement, JoinPoint thisJoinPoint) throws Throwable {
        String returnObject = (String) proceedingJoinPoint.proceed(new Object[]{ targetObject, regex, replacement});

        return returnObject;
    }



Same aspect written using AJ file instead of Java works as expected. Using Java and annotations however executes proceed on a wrong object.
Comment 1 Andrew Clement CLA 2016-11-17 19:45:40 EST
It isn't so much that it changes the order. It is that although you didn't bind 'this' because 'this' is different to 'target' at a call joinpoint it expects you to pass this  as well, in position 0 of the array. The code will work if you do that.

However, it should be required, this is a bug but it does seem to be a very long standing one, I'm surprised people are not tripping over it more regularly but perhaps binding only target on join points with differing this and target is unusual (I mean if doing that you would be better using execution() and binding 'this' to advise fewer places).

So I need to make a change to the AspectJ runtime, which I don't do lightly but think I must for this problem. Also fixing this breaks the fix for bug 288712, so I've re-fixed that in a better way. Thanks for the code snippet, always helpful. I've created many variants around this scheme (binding this/target, passing them through, changing them, etc) and they all pass with the proposed changes.