redesign of making members used in around bodies accessible

this design is confiined to the aspect containing the around entirely which
makes for cleaner implementation issues and lets things like super calls
be implemented correctly.

org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AccessForInlineVisitor.java

@@ -13,64 +13,73 @@
 
 package org.aspectj.ajdt.internal.compiler.ast;
 
-import java.util.Arrays;
+import java.util.*;
 
 import org.aspectj.ajdt.internal.compiler.lookup.*;
-import org.aspectj.ajdt.internal.compiler.lookup.PrivilegedHandler;
 import org.aspectj.weaver.*;
-import org.aspectj.weaver.ShadowMunger;
-import org.eclipse.jdt.internal.compiler.AbstractSyntaxTreeVisitorAdapter;
+import org.eclipse.jdt.internal.compiler.*;
 import org.eclipse.jdt.internal.compiler.ast.*;
 import org.eclipse.jdt.internal.compiler.lookup.*;
 
 /**
- * Takes a method that already has the three extra parameters
- * thisJoinPointStaticPart, thisJoinPoint and thisEnclosingJoinPointStaticPart
+ * Walks the body of around advice
+ * 
+ * Makes sure that all member accesses are to public members.  Will
+ * convert to use access methods when needed to ensure that.  This
+ * makes it much simpler (and more modular) to inline the body of
+ * an around.
+ * 
+ * ??? constructors are handled different and require access to the
+ * target type.  changes to org.eclipse.jdt.internal.compiler.ast.AllocationExpression
+ * would be required to fix this issue.
+ * 
+ * @author Jim Hugunin
  */
 
 public class AccessForInlineVisitor extends AbstractSyntaxTreeVisitorAdapter {
 	PrivilegedHandler handler;
-	EclipseWorld world;
-	public AccessForInlineVisitor(EclipseWorld world, PrivilegedHandler handler) {
-		this.world = world;
+	AspectDeclaration inAspect;
+	EclipseWorld world; // alias for inAspect.world
+	
+	public AccessForInlineVisitor(AspectDeclaration inAspect, PrivilegedHandler handler) {
+		this.inAspect = inAspect;
+		this.world = inAspect.world;
 		this.handler = handler;
 	}
 	
+	
 	public void endVisit(SingleNameReference ref, BlockScope scope) {
 		if (ref.binding instanceof FieldBinding) {
-			FieldBinding fieldBinding = (FieldBinding)ref.binding;
-			makePublic(fieldBinding.declaringClass);
-			if (isPublic(fieldBinding)) return;
-			ref.binding = handler.getPrivilegedAccessField(fieldBinding, ref);
+			ref.binding = getAccessibleField((FieldBinding)ref.binding);
 		}
 	}
 
 	public void endVisit(QualifiedNameReference ref, BlockScope scope) {
 		if (ref.binding instanceof FieldBinding) {
-			FieldBinding fieldBinding = (FieldBinding)ref.binding;
-			makePublic(fieldBinding.declaringClass);
-			if (isPublic(fieldBinding)) return;
-			ref.binding = handler.getPrivilegedAccessField(fieldBinding, ref);
+			ref.binding = getAccessibleField((FieldBinding)ref.binding);
 		}
 	}
 
 	public void endVisit(FieldReference ref, BlockScope scope) {
 		if (ref.binding instanceof FieldBinding) {
-			FieldBinding fieldBinding = (FieldBinding)ref.binding;
-			makePublic(fieldBinding.declaringClass);
-			if (isPublic(fieldBinding)) return;
-			ref.binding = handler.getPrivilegedAccessField(fieldBinding, ref);
+			ref.binding = getAccessibleField((FieldBinding)ref.binding);
 		}
 	}
 	public void endVisit(MessageSend send, BlockScope scope) {
 		if (send instanceof Proceed) return;
 		if (send.binding == null) return;
-		if (isPublic(send.binding)) return;
-		makePublic(send.binding.declaringClass);
-		send.binding = send.codegenBinding = handler.getPrivilegedAccessMethod(send.binding, send);
+		
+		if (send.isSuperAccess() && !send.binding.isStatic()) {
+			send.receiver = new ThisReference();
+			send.binding = send.codegenBinding = 
+				getSuperAccessMethod((MethodBinding)send.binding);
+		} else if (!isPublic(send.binding)) {
+			send.syntheticAccessor = getAccessibleMethod((MethodBinding)send.binding);
+		}
 	}
 	public void endVisit(AllocationExpression send, BlockScope scope) {
 		if (send.binding == null) return;
+		//XXX TBD
 		if (isPublic(send.binding)) return;
 		makePublic(send.binding.declaringClass);
 		send.binding = handler.getPrivilegedAccessMethod(send.binding, send);
@@ -89,6 +98,56 @@ public class AccessForInlineVisitor extends AbstractSyntaxTreeVisitorAdapter {
 		makePublic(ref.binding);
 	}
 	
+	private FieldBinding getAccessibleField(FieldBinding binding) {
+		if (!binding.isValidBinding()) return binding;
+		
+		makePublic(binding.declaringClass);
+		if (isPublic(binding)) return binding;
+		if (binding instanceof PrivilegedFieldBinding) return binding;
+		if (binding instanceof InterTypeFieldBinding) return binding;
+
+		if (binding.isPrivate() &&  binding.declaringClass != inAspect.binding) {
+			binding.modifiers = AstUtil.makePackageVisible(binding.modifiers);
+		}
+		
+		ResolvedMember m = world.makeResolvedMember(binding);
+		if (inAspect.accessForInline.containsKey(m)) return (FieldBinding)inAspect.accessForInline.get(m);
+		FieldBinding ret = new InlineAccessFieldBinding(inAspect, binding);
+		inAspect.accessForInline.put(m, ret);
+		return ret;
+	}
+	
+	private MethodBinding getAccessibleMethod(MethodBinding binding) {
+		if (!binding.isValidBinding()) return binding;
+		
+		makePublic(binding.declaringClass);  //???
+		if (isPublic(binding)) return binding;
+		if (binding instanceof InterTypeMethodBinding) return binding;
+
+		if (binding.isPrivate() &&  binding.declaringClass != inAspect.binding) {
+			binding.modifiers = AstUtil.makePackageVisible(binding.modifiers);
+		}
+
+		
+		ResolvedMember m = world.makeResolvedMember(binding);
+		if (inAspect.accessForInline.containsKey(m)) return (MethodBinding)inAspect.accessForInline.get(m);
+		MethodBinding ret = world.makeMethodBinding(
+			AjcMemberMaker.inlineAccessMethodForMethod(inAspect.typeX, m)
+			);
+		inAspect.accessForInline.put(m, ret);
+		return ret;
+	}
+	
+	private MethodBinding getSuperAccessMethod(MethodBinding binding) {
+		ResolvedMember m = world.makeResolvedMember(binding);
+		if (inAspect.superAccessForInline.containsKey(m)) return (MethodBinding)inAspect.superAccessForInline.get(m);
+		MethodBinding ret = world.makeMethodBinding(
+			AjcMemberMaker.superAccessMethod(inAspect.typeX, m)
+			);
+		inAspect.superAccessForInline.put(m, ret);
+		return ret;
+	}
+	
 	private boolean isPublic(FieldBinding fieldBinding) {
 		// these are always effectively public to the inliner
 		if (fieldBinding instanceof InterTypeFieldBinding) return true;

org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AdviceDeclaration.java

@@ -138,10 +138,10 @@ public class AdviceDeclaration extends MethodDeclaration {
 				}
 			}
 			
+			// ??? should reorganize into AspectDeclaration
 			// if we have proceed in inners we won't ever be inlined so the code below is unneeded
 			if (!proceedInInners) {
 				PrivilegedHandler handler = (PrivilegedHandler)upperScope.referenceContext.binding.privilegedHandler;
-				//XXX timings is odd here
 				if (handler == null) {
 					handler = new PrivilegedHandler((AspectDeclaration)upperScope.referenceContext);
 					upperScope.referenceContext.binding.privilegedHandler = handler;
@@ -149,7 +149,7 @@ public class AdviceDeclaration extends MethodDeclaration {
 				
 				this.traverse(new MakeDeclsPublicVisitor(), (ClassScope)null);
 				
-				AccessForInlineVisitor v = new AccessForInlineVisitor(world, handler);
+				AccessForInlineVisitor v = new AccessForInlineVisitor((AspectDeclaration)upperScope.referenceContext, handler);
 				this.traverse(v, (ClassScope) null);
 			}
 		}

org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AspectDeclaration.java

@@ -14,6 +14,7 @@
 package org.aspectj.ajdt.internal.compiler.ast;
 
 import java.lang.reflect.*;
+import java.util.*;
 
 import org.aspectj.ajdt.internal.compiler.lookup.*;
 import org.aspectj.weaver.*;
@@ -32,7 +33,10 @@ public class AspectDeclaration extends MemberTypeDeclaration {
 	public PerClause perClause;
 	public ResolvedMember aspectOfMethod;
 	public ResolvedMember hasAspectMethod;
-	
+
+
+	public Map accessForInline = new HashMap();
+	public Map superAccessForInline = new HashMap();
 	
 	public boolean isPrivileged;
 	
@@ -195,6 +199,8 @@ public class AspectDeclaration extends MemberTypeDeclaration {
 	protected void generateAttributes(ClassFile classFile) {		
 		if (!isAbstract()) generatePerSupportMembers(classFile);
 		
+		generateInlineAccessMembers(classFile);
+		
 		classFile.extraAttributes.add(
 			new EclipseAttributeAdapter(new AjAttribute.Aspect(perClause)));
 			
@@ -212,6 +218,18 @@ public class AspectDeclaration extends MemberTypeDeclaration {
 
 		super.generateAttributes(classFile);		
 	}
+	
+	private void generateInlineAccessMembers(ClassFile classFile) {
+		for (Iterator i = superAccessForInline.entrySet().iterator(); i.hasNext(); ) {
+			Map.Entry e = (Map.Entry)i.next();
+			generateSuperAccessMethod(classFile, (MethodBinding)e.getValue(), (ResolvedMember)e.getKey());
+		}
+		for (Iterator i = accessForInline.entrySet().iterator(); i.hasNext(); ) {
+			Map.Entry e = (Map.Entry)i.next();
+			generateInlineAccessMethod(classFile, (Binding)e.getValue(), (ResolvedMember)e.getKey());
+		}
+	}
+
 
 	private void generatePerSupportMembers(ClassFile classFile) {
 		if (isAbstract()) return;
@@ -553,6 +571,87 @@ public class AspectDeclaration extends MemberTypeDeclaration {
 
 	}
 	
+	private void generateSuperAccessMethod(ClassFile classFile, final MethodBinding accessMethod, final ResolvedMember method) {
+		generateMethod(classFile, accessMethod, 
+		new BodyGenerator() {
+			public void generate(CodeStream codeStream) {
+				// body starts here
+				codeStream.aload_0();
+				AstUtil.generateParameterLoads(accessMethod.parameters, codeStream);
+				codeStream.invokespecial(
+					world.makeMethodBinding(method));
+				AstUtil.generateReturn(accessMethod.returnType, codeStream);
+				// body ends here
+			}});
+
+	}
+	
+
+	private void generateInlineAccessMethod(ClassFile classFile, final Binding binding, final ResolvedMember member) {
+		if (binding instanceof InlineAccessFieldBinding) {
+			generateInlineAccessors(classFile, (InlineAccessFieldBinding)binding, member);
+		} else {
+			generateInlineAccessMethod(classFile, (MethodBinding)binding, member);
+		}
+	}
+	
+	private void generateInlineAccessors(ClassFile classFile, final InlineAccessFieldBinding accessField, final ResolvedMember field) {
+		final FieldBinding fieldBinding = world.makeFieldBinding(field);
+		generateMethod(classFile, accessField.reader, 
+		new BodyGenerator() {
+			public void generate(CodeStream codeStream) {
+				// body starts here
+				if (field.isStatic()) {
+					codeStream.getstatic(fieldBinding);
+				} else {
+					codeStream.aload_0();
+					codeStream.getfield(fieldBinding);
+				}
+					
+				AstUtil.generateReturn(accessField.reader.returnType, codeStream);
+				// body ends here
+			}});
+			
+		generateMethod(classFile, accessField.writer, 
+		new BodyGenerator() {
+			public void generate(CodeStream codeStream) {
+				// body starts here
+				if (field.isStatic()) {
+					codeStream.load(fieldBinding.type, 0);
+					codeStream.putstatic(fieldBinding);
+				} else {
+					codeStream.aload_0();
+					codeStream.load(fieldBinding.type, 1);
+					codeStream.putfield(fieldBinding);
+				}	
+					
+				codeStream.return_();
+				// body ends here
+			}});
+
+	}
+	
+
+	private void generateInlineAccessMethod(ClassFile classFile, final MethodBinding accessMethod, final ResolvedMember method) {
+		generateMethod(classFile, accessMethod, 
+		new BodyGenerator() {
+			public void generate(CodeStream codeStream) {
+				// body starts here
+				
+				AstUtil.generateParameterLoads(accessMethod.parameters, codeStream);
+				
+				if (method.isStatic()) {
+					codeStream.invokestatic(world.makeMethodBinding(method));
+				} else {
+				    codeStream.invokevirtual(world.makeMethodBinding(method));
+				}
+					
+				AstUtil.generateReturn(accessMethod.returnType, codeStream);
+				// body ends here
+			}});
+	}
+	
+
 	
 	private PerClause.Kind lookupPerClauseKind(ReferenceBinding binding) {
 		if (binding instanceof SourceTypeBinding && !(binding instanceof BinaryTypeBinding)) {

org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AstUtil.java

@@ -132,8 +132,12 @@ public class AstUtil {
 	}
 	
 	public static int makePublic(int modifiers) {
+		return makePackageVisible(modifiers) | IConstants.AccPublic;
+	}
+
+	public static int makePackageVisible(int modifiers) {
 		modifiers &= ~(IConstants.AccPublic | IConstants.AccPrivate | IConstants.AccProtected);
-		return modifiers | IConstants.AccPublic;
+		return modifiers;
 	}
 
 	public static CompilationUnitScope getCompilationUnitScope(Scope scope) {
@@ -144,6 +148,17 @@ public class AstUtil {
 	}
 	
 	
+	public static void generateParameterLoads(TypeBinding[] parameters, CodeStream codeStream) {
+		int paramIndex = 0;
+		int varIndex = 0;
+		while (paramIndex < parameters.length) {
+			TypeBinding param = parameters[paramIndex++];
+			codeStream.load(param, varIndex);
+			varIndex += slotsNeeded(param);
+		}
+	}
+
+	
 	public static void generateReturn(TypeBinding returnType, CodeStream codeStream) {
 		if (returnType.id == TypeIds.T_void) {
 			codeStream.return_();

org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/SuperFixerVisitor.java

@@ -50,7 +50,6 @@ public class SuperFixerVisitor extends AbstractSyntaxTreeVisitorAdapter {
 			return;
 		}
 		char[] accessName;
-		boolean isSuper;
 		if (call.isSuperAccess() && !call.binding.isStatic()) {
 			call.receiver = new ThisReference();
 			accessName =

org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/InlineAccessFieldBinding.java

@@ -0,0 +1,79 @@
+/* *******************************************************************
+ * Copyright (c) 2002 Palo Alto Research Center, Incorporated (PARC).
+ * All rights reserved. 
+ * This program and the accompanying materials are made available 
+ * under the terms of the Common Public License v1.0 
+ * which accompanies this distribution and is available at 
+ * http://www.eclipse.org/legal/cpl-v10.html 
+ *  
+ * Contributors: 
+ *     PARC     initial implementation 
+ * ******************************************************************/
+
+
+package org.aspectj.ajdt.internal.compiler.lookup;
+
+import java.lang.reflect.Modifier;
+
+import org.aspectj.ajdt.internal.compiler.ast.*;
+import org.aspectj.ajdt.internal.compiler.ast.AstUtil;
+import org.aspectj.weaver.*;
+import org.aspectj.weaver.NameMangler;
+import org.eclipse.jdt.internal.compiler.ast.*;
+import org.eclipse.jdt.internal.compiler.ast.FieldDeclaration;
+import org.eclipse.jdt.internal.compiler.impl.Constant;
+import org.eclipse.jdt.internal.compiler.lookup.*;
+import org.eclipse.jdt.internal.compiler.util.CharOperation;
+
+
+/**
+ * Used for field references within the body of an around advice
+ * to force the use of public access methods.  This makes it possible
+ * for around advice to be inlined into any shadow to which it applies.
+ * 
+ * ??? this is very similar to PrivilegedFieldBinding and is somewhat related
+ *     to InterTypeFieldBinding.  Maybe they have a common supertype?
+ * 
+ * @author Jim Hugunin
+ */
+public class InlineAccessFieldBinding extends FieldBinding {
+	public SimpleSyntheticAccessMethodBinding reader;
+	public SimpleSyntheticAccessMethodBinding writer;
+	
+	
+	public FieldBinding baseField;
+	
+	public InlineAccessFieldBinding(AspectDeclaration inAspect, FieldBinding baseField) {
+		super(baseField, baseField.declaringClass);
+
+		this.reader = new SimpleSyntheticAccessMethodBinding(
+			inAspect.world.makeMethodBinding(
+				AjcMemberMaker.inlineAccessMethodForFieldGet(
+					inAspect.typeX, inAspect.world.makeResolvedMember(baseField)
+			)));
+		this.writer = new SimpleSyntheticAccessMethodBinding(inAspect.world.makeMethodBinding(
+				AjcMemberMaker.inlineAccessMethodForFieldSet(
+					inAspect.typeX, inAspect.world.makeResolvedMember(baseField)
+			)));
+			
+		this.constant = AstNode.NotAConstant;
+		this.baseField = baseField;
+	}
+
+
+	public boolean canBeSeenBy(TypeBinding receiverType, InvocationSite invocationSite, Scope scope) {
+		return true;
+	}
+
+	public SyntheticAccessMethodBinding getAccessMethod(boolean isReadAccess) {
+		if (isReadAccess) return reader;
+		else return writer;
+	}
+	
+	public boolean alwaysNeedsAccessMethod(boolean isReadAccess) { return true; }
+
+	public FieldBinding getFieldBindingForLookup() { return baseField; }
+
+
+	public String toString() { return "InlineAccess(" + baseField + ")"; }
+}

org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/PrivilegedHandler.java

@@ -82,7 +82,6 @@ public class PrivilegedHandler implements IPrivilegedHandler {
 			}
 		}
 	}
-
 	
 	public ResolvedMember[] getMembers() {
 		Collection m = accessors.keySet();
@@ -94,7 +93,4 @@ public class PrivilegedHandler implements IPrivilegedHandler {
 		}
 		return ret;
 	}
-
-
-
 }

weaver/src/org/aspectj/weaver/AjcMemberMaker.java

@@ -215,7 +215,68 @@ public class AjcMemberMaker {
 			sig);
 	}
 	
+	// --- inline accessors
+	//??? can eclipse handle a transform this weird without putting synthetics into the mix
+	public static ResolvedMember superAccessMethod(TypeX baseType, ResolvedMember method) {
+		return new ResolvedMember(Member.METHOD,
+			baseType,
+			Modifier.PUBLIC,
+			method.getReturnType(),
+			NameMangler.superDispatchMethod(baseType, method.getName()),
+			method.getParameterTypes());
+			//XXX needs thrown exceptions to be correct
+	}
 	
+	public static ResolvedMember inlineAccessMethodForMethod(TypeX aspectType, ResolvedMember method) {
+		TypeX[] paramTypes = method.getParameterTypes();
+		if (!method.isStatic()) {
+			paramTypes = TypeX.insert(method.getDeclaringType(), paramTypes);
+		}
+		return new ResolvedMember(Member.METHOD,
+			aspectType,
+			PUBLIC_STATIC, //??? what about privileged and super access
+						   //???Modifier.PUBLIC | (method.isStatic() ? Modifier.STATIC : 0),
+			method.getReturnType(),
+			
+			NameMangler.inlineAccessMethodForMethod(method.getName(),
+												method.getDeclaringType(), aspectType),
+			paramTypes);
+			//XXX needs thrown exceptions to be correct
+	}
+	
+	public static ResolvedMember inlineAccessMethodForFieldGet(TypeX aspectType, Member field) {
+		String sig;
+		if (field.isStatic()) {
+			sig = "()" + field.getReturnType().getSignature();
+		} else {
+			sig = "(" + field.getDeclaringType().getSignature() + ")" + field.getReturnType().getSignature();
+		}
+		
+		return new ResolvedMember(Member.METHOD,
+			aspectType,
+			PUBLIC_STATIC, //Modifier.PUBLIC | (field.isStatic() ? Modifier.STATIC : 0),
+			NameMangler.inlineAccessMethodForFieldGet(field.getName(),
+												field.getDeclaringType(), aspectType), 
+			sig);
+	}
+	
+	public static ResolvedMember inlineAccessMethodForFieldSet(TypeX aspectType, Member field) {
+		String sig;
+		if (field.isStatic()) {
+			sig = "(" + field.getReturnType().getSignature() + ")V";
+		} else {
+			sig = "(" + field.getDeclaringType().getSignature() + field.getReturnType().getSignature() + ")V";
+		}
+		
+		return new ResolvedMember(Member.METHOD,
+			aspectType,
+			PUBLIC_STATIC, //Modifier.PUBLIC | (field.isStatic() ? Modifier.STATIC : 0),
+			NameMangler.inlineAccessMethodForFieldSet(field.getName(),
+												field.getDeclaringType(), aspectType), 
+			sig);
+	}
+	
+
 
 	// --- runtimeLibrary api stuff
 

weaver/src/org/aspectj/weaver/NameMangler.java

@@ -74,6 +74,23 @@ public class NameMangler {
 	
 	
 	
+	public static String inlineAccessMethodForMethod(String name, TypeX objectType, TypeX aspectType) {
+		return makeName("inlineAccessMethod", aspectType.getNameAsIdentifier(),
+					objectType.getNameAsIdentifier(), name);
+	}
+	
+	public static String inlineAccessMethodForFieldGet(String name, TypeX objectType, TypeX aspectType) {
+		return makeName("inlineAccessFieldGet", aspectType.getNameAsIdentifier(),
+					objectType.getNameAsIdentifier(), name);
+	}
+	
+	public static String inlineAccessMethodForFieldSet(String name, TypeX objectType, TypeX aspectType) {
+		return makeName("inlineAccessFieldSet", aspectType.getNameAsIdentifier(),
+					objectType.getNameAsIdentifier(), name);
+	}
+	
+	
+	
 	/**
 	 * The name of methods corresponding to advice declarations
 	 */