From 4f3e9900912b98fca25df24fac521b0986d6891e Mon Sep 17 00:00:00 2001 From: Alexander Kriegisch Date: Wed, 10 Apr 2024 13:22:04 +0200 Subject: [PATCH] JoinPointImpl minor structural refactoring No functionality was changed. Signed-off-by: Alexander Kriegisch --- .../runtime/reflect/JoinPointImpl.java | 168 ++++++++---------- 1 file changed, 79 insertions(+), 89 deletions(-) diff --git a/runtime/src/main/java/org/aspectj/runtime/reflect/JoinPointImpl.java b/runtime/src/main/java/org/aspectj/runtime/reflect/JoinPointImpl.java index 01def8451..9a8d1e65d 100644 --- a/runtime/src/main/java/org/aspectj/runtime/reflect/JoinPointImpl.java +++ b/runtime/src/main/java/org/aspectj/runtime/reflect/JoinPointImpl.java @@ -13,7 +13,6 @@ package org.aspectj.runtime.reflect; -import org.aspectj.lang.JoinPoint; import org.aspectj.lang.ProceedingJoinPoint; import org.aspectj.lang.Signature; import org.aspectj.lang.reflect.SourceLocation; @@ -23,11 +22,11 @@ import java.util.ArrayList; import java.util.List; class JoinPointImpl implements ProceedingJoinPoint { - static class StaticPartImpl implements JoinPoint.StaticPart { + static class StaticPartImpl implements StaticPart { String kind; Signature signature; SourceLocation sourceLocation; - private int id; + private final int id; public StaticPartImpl(int id, String kind, Signature signature, SourceLocation sourceLocation) { this.kind = kind; @@ -53,12 +52,7 @@ class JoinPointImpl implements ProceedingJoinPoint { } String toString(StringMaker sm) { - StringBuilder buf = new StringBuilder(); - buf.append(sm.makeKindName(getKind())); - buf.append("("); - buf.append(((SignatureImpl) getSignature()).toString(sm)); - buf.append(")"); - return buf.toString(); + return sm.makeKindName(getKind()) + "(" + ((SignatureImpl) getSignature()).toString(sm) + ")"; } public final String toString() { @@ -83,9 +77,9 @@ class JoinPointImpl implements ProceedingJoinPoint { Object _this; Object target; Object[] args; - org.aspectj.lang.JoinPoint.StaticPart staticPart; + StaticPart staticPart; - public JoinPointImpl(org.aspectj.lang.JoinPoint.StaticPart staticPart, Object _this, Object target, Object[] args) { + public JoinPointImpl(StaticPart staticPart, Object _this, Object target, Object[] args) { this.staticPart = staticPart; this._this = _this; this.target = target; @@ -101,15 +95,14 @@ class JoinPointImpl implements ProceedingJoinPoint { } public Object[] getArgs() { - if (args == null) { + if (args == null) args = new Object[0]; - } Object[] argsCopy = new Object[args.length]; System.arraycopy(args, 0, argsCopy, 0, args.length); return argsCopy; } - public org.aspectj.lang.JoinPoint.StaticPart getStaticPart() { + public StaticPart getStaticPart() { return staticPart; } @@ -150,9 +143,8 @@ class JoinPointImpl implements ProceedingJoinPoint { public void stack$AroundClosure(AroundClosure arc) { // If input parameter arc is null this is the 'unlink' call from AroundClosure - if (arcs == null) { + if (arcs == null) arcs = new ArrayList<>(); - } if (arc == null) { int newIndex = arcIndex.get() - 1; if (newIndex > -1) @@ -169,11 +161,7 @@ class JoinPointImpl implements ProceedingJoinPoint { public Object proceed() throws Throwable { // when called from a before advice, but be a no-op if (arcs == null) { - if (arc == null) { - return null; - } else { - return arc.run(arc.getState()); - } + return arc == null ? null : arc.run(arc.getState()); } else { final AroundClosure ac = arcs.get(arcIndex.get()); return ac.run(ac.getState()); @@ -184,82 +172,84 @@ class JoinPointImpl implements ProceedingJoinPoint { // when called from a before advice, but be a no-op AroundClosure ac = arcs == null ? arc : arcs.get(arcIndex.get()); - if (ac == null) { + if (ac == null) return null; - } else { - // Based on the bit flags in the AroundClosure we can determine what to - // expect in the adviceBindings array. We may or may not be expecting - // the first value to be a new this or a new target... (see pr126167) - int flags = ac.getFlags(); - boolean unset = (flags & 0x100000) != 0; - boolean thisTargetTheSame = (flags & 0x010000) != 0; - boolean hasThis = (flags & 0x001000) != 0; - boolean bindsThis = (flags & 0x000100) != 0; - boolean hasTarget = (flags & 0x000010) != 0; - boolean bindsTarget = (flags & 0x000001) != 0; - - // state is always consistent with caller?,callee?,formals...,jp - Object[] state = ac.getState(); - - // these next two numbers can differ because some join points have a this and - // target that are the same (eg. call) - and yet you can bind this and target - // separately. - - // In the state array, [0] may be this, [1] may be target - - int firstArgumentIndexIntoAdviceBindings = 0; - int firstArgumentIndexIntoState = 0; - firstArgumentIndexIntoState += (hasThis ? 1 : 0); - firstArgumentIndexIntoState += (hasTarget && !thisTargetTheSame ? 1 : 0); - if (hasThis) { - if (bindsThis) { - // replace [0] (this) - firstArgumentIndexIntoAdviceBindings = 1; - state[0] = adviceBindings[0]; - } else { - // leave state[0] alone, its OK - } + + // Based on the bit flags in the AroundClosure we can determine what to + // expect in the adviceBindings array. We may or may not be expecting + // the first value to be a new this or a new target... (see pr126167) + int flags = ac.getFlags(); + boolean unset = (flags & 0x100000) != 0; + boolean thisTargetTheSame = (flags & 0x010000) != 0; + boolean hasThis = (flags & 0x001000) != 0; + boolean bindsThis = (flags & 0x000100) != 0; + boolean hasTarget = (flags & 0x000010) != 0; + boolean bindsTarget = (flags & 0x000001) != 0; + + // state is always consistent with caller?,callee?,formals...,jp + Object[] state = ac.getState(); + + // these next two numbers can differ because some join points have a this and + // target that are the same (eg. call) - and yet you can bind this and target + // separately. + + // In the state array, [0] may be this, [1] may be target + + int firstArgumentIndexIntoAdviceBindings = 0; + int firstArgumentIndexIntoState = 0; + firstArgumentIndexIntoState += (hasThis ? 1 : 0); + firstArgumentIndexIntoState += (hasTarget && !thisTargetTheSame ? 1 : 0); + if (hasThis) { + if (bindsThis) { + // replace [0] (this) + firstArgumentIndexIntoAdviceBindings = 1; + state[0] = adviceBindings[0]; + } else { + // leave state[0] alone, its OK } - if (hasTarget) { - if (bindsTarget) { - if (thisTargetTheSame) { - // this and target are the same so replace state[0] - firstArgumentIndexIntoAdviceBindings = 1 + (bindsThis ? 1 : 0); - state[0] = adviceBindings[(bindsThis ? 1 : 0)]; - } else { - // need to replace the target, and it is different to this, whether - // that means replacing state[0] or state[1] depends on whether - // the join point has a this - - // This previous variant doesn't seem to cope with only binding target at a joinpoint - // which has both this and target. It forces you to supply this even if you didn't bind - // it. - // firstArgumentIndexIntoAdviceBindings = (hasThis ? 1 : 0) + 1; - // state[hasThis ? 1 : 0] = adviceBindings[hasThis ? 1 : 0]; - - int targetPositionInAdviceBindings = (hasThis && bindsThis) ? 1 : 0; - firstArgumentIndexIntoAdviceBindings = ((hasThis&&bindsThis)?1:0)+((hasTarget&&bindsTarget&&!thisTargetTheSame)?1:0); - state[hasThis ? 1 : 0] = adviceBindings[targetPositionInAdviceBindings]; - } + } + if (hasTarget) { + if (bindsTarget) { + if (thisTargetTheSame) { + // this and target are the same so replace state[0] + firstArgumentIndexIntoAdviceBindings = 1 + (bindsThis ? 1 : 0); + state[0] = adviceBindings[(bindsThis ? 1 : 0)]; } else { - // leave state[0]/state[1] alone, they are OK + // need to replace the target, and it is different to this, whether + // that means replacing state[0] or state[1] depends on whether + // the join point has a this + + // This previous variant doesn't seem to cope with only binding target at a joinpoint which has both this and + // target. It forces you to supply this even if you didn't bind it. + /* + firstArgumentIndexIntoAdviceBindings = (hasThis ? 1 : 0) + 1; + state[hasThis ? 1 : 0] = adviceBindings[hasThis ? 1 : 0]; + */ + + int targetPositionInAdviceBindings = (hasThis && bindsThis) ? 1 : 0; + firstArgumentIndexIntoAdviceBindings = ((hasThis&&bindsThis)?1:0)+((hasTarget&&bindsTarget&&!thisTargetTheSame)?1:0); + state[hasThis ? 1 : 0] = adviceBindings[targetPositionInAdviceBindings]; } + } else { + // leave state[0]/state[1] alone, they are OK } + } - // copy the rest across - for (int i = firstArgumentIndexIntoAdviceBindings; i < adviceBindings.length; i++) { - state[firstArgumentIndexIntoState + (i - firstArgumentIndexIntoAdviceBindings)] = adviceBindings[i]; - } + // copy the rest across + for (int i = firstArgumentIndexIntoAdviceBindings; i < adviceBindings.length; i++) { + state[firstArgumentIndexIntoState + (i - firstArgumentIndexIntoAdviceBindings)] = adviceBindings[i]; + } - // old code that did this, didnt allow this/target overriding - // for (int i = state.length-2; i >= 0; i--) { - // int formalIndex = (adviceBindings.length - 1) - (state.length-2) + i; - // if (formalIndex >= 0 && formalIndex < adviceBindings.length) { - // state[i] = adviceBindings[formalIndex]; - // } - // } - return ac.run(state); + // old code that did this, didnt allow this/target overriding + /* + for (int i = state.length - 2; i >= 0; i--) { + int formalIndex = (adviceBindings.length - 1) - (state.length - 2) + i; + if (formalIndex >= 0 && formalIndex < adviceBindings.length) { + state[i] = adviceBindings[formalIndex]; + } } + */ + return ac.run(state); } } -- 2.39.5