From c86b373e28c6b0c77c90eebddf693db9df8f9ccb Mon Sep 17 00:00:00 2001 From: jhugunin Date: Thu, 19 Dec 2002 18:50:54 +0000 Subject: [PATCH] fixed error handling for illegal overriding with inter-type members --- .../ast/InterTypeConstructorDeclaration.java | 2 +- .../compiler/ast/InterTypeDeclaration.java | 9 +++- .../ast/InterTypeFieldDeclaration.java | 2 +- .../ast/InterTypeMethodDeclaration.java | 2 +- .../compiler/lookup/EclipseObjectType.java | 6 ++- .../src/org/aspectj/weaver/ResolvedTypeX.java | 47 ++++++++++++++----- weaver/src/org/aspectj/weaver/World.java | 21 ++++++++- 7 files changed, 70 insertions(+), 19 deletions(-) diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeConstructorDeclaration.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeConstructorDeclaration.java index 286161689..d32e2c009 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeConstructorDeclaration.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeConstructorDeclaration.java @@ -210,7 +210,7 @@ public class InterTypeConstructorDeclaration extends InterTypeDeclaration { NewConstructorTypeMunger myMunger = new NewConstructorTypeMunger(signature, syntheticInterMember, null, null); - this.munger = myMunger; + setMunger(myMunger); this.selector = binding.selector = NameMangler.postIntroducedConstructor( diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeDeclaration.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeDeclaration.java index a9a73337a..fba705bc6 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeDeclaration.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeDeclaration.java @@ -17,6 +17,7 @@ import java.lang.reflect.Modifier; import java.util.*; import org.aspectj.ajdt.internal.compiler.lookup.*; +import org.aspectj.ajdt.internal.core.builder.EclipseSourceContext; import org.aspectj.weaver.*; import org.eclipse.jdt.internal.compiler.*; import org.eclipse.jdt.internal.compiler.CompilationResult; @@ -113,8 +114,14 @@ public abstract class InterTypeDeclaration extends MethodDeclaration { return l; } - protected int generateInfoAttributes(ClassFile classFile) { + protected void setMunger(ResolvedTypeMunger munger) { munger.getSignature().setPosition(sourceStart, sourceEnd); + munger.getSignature().setSourceContext(new EclipseSourceContext(compilationResult)); + this.munger = munger; + } + + protected int generateInfoAttributes(ClassFile classFile) { + //munger.getSignature().setPosition(sourceStart, sourceEnd); //System.out.println("generating effective for " + this); List l;; diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeFieldDeclaration.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeFieldDeclaration.java index 0ed6b63c9..86d5c9c8b 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeFieldDeclaration.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeFieldDeclaration.java @@ -123,7 +123,7 @@ public class InterTypeFieldDeclaration extends InterTypeDeclaration { new String(declaredSelector), TypeX.NONE); NewFieldTypeMunger myMunger = new NewFieldTypeMunger(sig, null); - this.munger = myMunger; + setMunger(myMunger); ResolvedTypeX aspectType = world.fromEclipse(classScope.referenceContext.binding); ResolvedMember me = myMunger.getInitMethod(aspectType); diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeMethodDeclaration.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeMethodDeclaration.java index 8b54aaf1e..899d5ec6b 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeMethodDeclaration.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/InterTypeMethodDeclaration.java @@ -81,7 +81,7 @@ public class InterTypeMethodDeclaration extends InterTypeDeclaration { EclipseWorld.fromBindings(binding.parameters)); NewMethodTypeMunger myMunger = new NewMethodTypeMunger(sig, null); - this.munger = myMunger; + setMunger(myMunger); ResolvedTypeX aspectType = world.fromEclipse(classScope.referenceContext.binding); ResolvedMember me = myMunger.getDispatchMethod(aspectType); diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseObjectType.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseObjectType.java index dde1a95bb..f0c37813b 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseObjectType.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseObjectType.java @@ -149,9 +149,11 @@ public class EclipseObjectType extends ResolvedTypeX.Name { } } - //XXX now check all inherited pointcuts to be sure that they're handled reasonably - + //now check all inherited pointcuts to be sure that they're handled reasonably + if (!isAspect()) return; +// for (Iterator i = getSuperclass().getFields(); ) +// XXX } diff --git a/weaver/src/org/aspectj/weaver/ResolvedTypeX.java b/weaver/src/org/aspectj/weaver/ResolvedTypeX.java index b23dbe58f..bcfd9e02b 100644 --- a/weaver/src/org/aspectj/weaver/ResolvedTypeX.java +++ b/weaver/src/org/aspectj/weaver/ResolvedTypeX.java @@ -16,6 +16,7 @@ package org.aspectj.weaver; import java.lang.reflect.Modifier; import java.util.*; +import org.aspectj.bridge.*; import org.aspectj.bridge.MessageUtil; import org.aspectj.weaver.bcel.BcelObjectType; import org.aspectj.weaver.patterns.*; @@ -273,7 +274,7 @@ public abstract class ResolvedTypeX extends TypeX { * We keep a hashSet of interfaces that we've visited so we don't spiral * out into 2^n land. */ - private Iterator getPointcuts() { + public Iterator getPointcuts() { final Iterators.Filter dupFilter = Iterators.dupFilter(); // same order as fields Iterators.Getter typeGetter = new Iterators.Getter() { @@ -840,9 +841,11 @@ public abstract class ResolvedTypeX extends TypeX { //System.err.println(" compare: " + c); if (c < 0) { // the existing munger dominates the new munger + checkLegalOverride(existingMunger.getSignature(), munger.getSignature()); return; } else if (c > 0) { // the new munger dominates the existing one + checkLegalOverride(munger.getSignature(), existingMunger.getSignature()); i.remove(); break; } else { @@ -866,11 +869,16 @@ public abstract class ResolvedTypeX extends TypeX { if (conflictingSignature(existingMember, munger.getSignature())) { //System.err.println("conflict: " + existingMember + " with " + munger); + //System.err.println(munger.getSourceLocation() + ", " + munger.getSignature() + ", " + munger.getSignature().getSourceLocation()); + if (isVisible(existingMember.getModifiers(), this, munger.getAspectType())) { int c = compareMemberPrecedence(sig, existingMember); //System.err.println(" c: " + c); - if (c < 0) return false; - else if (c > 0) { + if (c < 0) { + checkLegalOverride(existingMember, munger.getSignature()); + return false; + } else if (c > 0) { + checkLegalOverride(munger.getSignature(), existingMember); //interTypeMungers.add(munger); //??? might need list of these overridden abstracts continue; @@ -891,11 +899,27 @@ public abstract class ResolvedTypeX extends TypeX { return true; } - - + public boolean checkLegalOverride(ResolvedMember parent, ResolvedMember child) { + if (!parent.getReturnType().equals(child.getReturnType())) { + world.showMessage(IMessage.ERROR, + "can't override " + parent + + " with " + child + " return types don't match", + child.getSourceLocation(), parent.getSourceLocation()); + return false; + } + if (isMoreVisible(child.getModifiers(), parent.getModifiers())) { + world.showMessage(IMessage.ERROR, + "can't override " + parent + + " with " + child + " visibility is reduced", + child.getSourceLocation(), parent.getSourceLocation()); + return false; + } + return true; + + } private int compareMemberPrecedence(ResolvedMember m1, ResolvedMember m2) { - if (!m1.getReturnType().equals(m2.getReturnType())) return 0; + //if (!m1.getReturnType().equals(m2.getReturnType())) return 0; if (Modifier.isAbstract(m1.getModifiers())) return -1; if (Modifier.isAbstract(m2.getModifiers())) return +1; @@ -905,20 +929,20 @@ public abstract class ResolvedTypeX extends TypeX { ResolvedTypeX t1 = m1.getDeclaringType().resolve(world); ResolvedTypeX t2 = m2.getDeclaringType().resolve(world); if (t1.isAssignableFrom(t2)) { - checkVisibility(m1.getModifiers(), m2.getModifiers()); //XXX needs to be before abstract return -1; } if (t2.isAssignableFrom(t1)) { - checkVisibility(m2.getModifiers(), m1.getModifiers()); return +1; } return 0; } - private void checkVisibility(int parentMods, int childMods) { - //childMods must be equal to or greater than parentMods visibility - + public static boolean isMoreVisible(int m1, int m2) { + if (Modifier.isPublic(m1)) return !Modifier.isPublic(m2); + else if (Modifier.isPrivate(m1)) return false; + else if (Modifier.isProtected(m1)) return !(Modifier.isPublic(m2) || Modifier.isProtected(m2)); + else return Modifier.isPrivate(m1); } @@ -971,5 +995,4 @@ public abstract class ResolvedTypeX extends TypeX { } return true; } - } diff --git a/weaver/src/org/aspectj/weaver/World.java b/weaver/src/org/aspectj/weaver/World.java index 99f4c6c18..d31b9e7a4 100644 --- a/weaver/src/org/aspectj/weaver/World.java +++ b/weaver/src/org/aspectj/weaver/World.java @@ -19,7 +19,7 @@ import org.aspectj.weaver.patterns.*; import org.aspectj.weaver.patterns.Pointcut; import org.aspectj.asm.StructureModel; import org.aspectj.bridge.*; -import org.aspectj.bridge.IMessageHandler; +import org.aspectj.bridge.IMessage.Kind; public abstract class World { protected IMessageHandler messageHandler = IMessageHandler.SYSTEM_ERR; @@ -223,6 +223,25 @@ public abstract class World { public void setMessageHandler(IMessageHandler messageHandler) { this.messageHandler = messageHandler; } + + public void showMessage( + Kind kind, + String message, + ISourceLocation loc1, + ISourceLocation loc2) + { + if (loc1 != null) { + messageHandler.handleMessage(new Message(message, kind, null, loc1)); + if (loc2 != null) { + messageHandler.handleMessage(new Message(message, kind, null, loc2)); + } + } else { + messageHandler.handleMessage(new Message(message, kind, null, loc2)); + } + } + + + // public void addDeclare(ResolvedTypeX onType, Declare declare, boolean forWeaving) { // // this is not extensible, oh well -- 2.39.5