PING I am -1 on the commit as it stands; please revert or fix
On 28 August 2015 at 01:43, sebb <seb...@gmail.com> wrote: > On 12 August 2015 at 07:32, <c...@apache.org> wrote: >> Author: chas >> Date: Wed Aug 12 06:32:41 2015 >> New Revision: 1695425 >> >> URL: http://svn.apache.org/r1695425 >> Log: >> BCEL-236: remove deprecated FieldOrMethod.getClassType(ConConstantPoolGen) > > I think this needs to be reverted or amended. > > The new method FieldOrMethod.getLoadClassType(ConstantPoolGen cpg) can > throw a ClassCastException. > See below. > >> Modified: >> >> commons/proper/bcel/trunk/src/main/java/org/apache/commons/bcel6/generic/FieldOrMethod.java >> >> commons/proper/bcel/trunk/src/main/java/org/apache/commons/bcel6/verifier/statics/Pass3aVerifier.java >> >> commons/proper/bcel/trunk/src/main/java/org/apache/commons/bcel6/verifier/structurals/InstConstraintVisitor.java >> >> Modified: >> commons/proper/bcel/trunk/src/main/java/org/apache/commons/bcel6/generic/FieldOrMethod.java >> URL: >> http://svn.apache.org/viewvc/commons/proper/bcel/trunk/src/main/java/org/apache/commons/bcel6/generic/FieldOrMethod.java?rev=1695425&r1=1695424&r2=1695425&view=diff >> ============================================================================== >> --- >> commons/proper/bcel/trunk/src/main/java/org/apache/commons/bcel6/generic/FieldOrMethod.java >> (original) >> +++ >> commons/proper/bcel/trunk/src/main/java/org/apache/commons/bcel6/generic/FieldOrMethod.java >> Wed Aug 12 06:32:41 2015 >> @@ -95,17 +95,6 @@ public abstract class FieldOrMethod exte >> } >> >> >> - /** @return type of the referenced class/interface >> - * @deprecated If the instruction references an array class, >> - * the ObjectType returned will be invalid. Use >> - * getReferenceType() instead. >> - */ >> - @Deprecated >> - public ObjectType getClassType( ConstantPoolGen cpg ) { >> - return ObjectType.getInstance(getClassName(cpg)); >> - } >> - >> - >> /** >> * Return the reference type representing the class, interface, >> * or array class referenced by the instruction. >> @@ -131,6 +120,6 @@ public abstract class FieldOrMethod exte >> /** @return type of the referenced class/interface >> */ >> public ObjectType getLoadClassType( ConstantPoolGen cpg ) { >> - return getClassType(cpg); >> + return (ObjectType)getReferenceType(cpg); > > ObjectType is not a subclass of ArrayType > > It does not seem right to hide the original reason for the deprecation this > way. > > The code should at least check the object type and throw a better > exception than CCE > And the Javadoc should make the pre-condition clear > >> } >> } >> >> Modified: >> commons/proper/bcel/trunk/src/main/java/org/apache/commons/bcel6/verifier/statics/Pass3aVerifier.java >> URL: >> http://svn.apache.org/viewvc/commons/proper/bcel/trunk/src/main/java/org/apache/commons/bcel6/verifier/statics/Pass3aVerifier.java?rev=1695425&r1=1695424&r2=1695425&view=diff >> ============================================================================== >> --- >> commons/proper/bcel/trunk/src/main/java/org/apache/commons/bcel6/verifier/statics/Pass3aVerifier.java >> (original) >> +++ >> commons/proper/bcel/trunk/src/main/java/org/apache/commons/bcel6/verifier/statics/Pass3aVerifier.java >> Wed Aug 12 06:32:41 2015 >> @@ -85,6 +85,7 @@ import org.apache.commons.bcel6.generic. >> import org.apache.commons.bcel6.generic.ObjectType; >> import org.apache.commons.bcel6.generic.PUTSTATIC; >> import org.apache.commons.bcel6.generic.RET; >> +import org.apache.commons.bcel6.generic.ReferenceType; >> import org.apache.commons.bcel6.generic.ReturnInstruction; >> import org.apache.commons.bcel6.generic.TABLESWITCH; >> import org.apache.commons.bcel6.generic.Type; >> @@ -543,6 +544,15 @@ public final class Pass3aVerifier extend >> } >> } >> >> + private ObjectType getObjectType(FieldInstruction o) { >> + ReferenceType rt = o.getReferenceType(cpg); >> + if(rt instanceof ObjectType) { >> + return (ObjectType)rt; >> + } >> + constraintViolated(o, "expecting ObjectType but got "+rt); >> + return null; >> + } > > Here we see that the code checks the return class, which is fine. > >> /** Checks if the constraints of operands of the said >> instruction(s) are satisfied. */ >> //getfield, putfield, getstatic, putstatic >> @Override >> @@ -555,8 +565,8 @@ public final class Pass3aVerifier extend >> } >> >> String field_name = o.getFieldName(cpg); >> - >> - JavaClass jc = >> Repository.lookupClass(o.getClassType(cpg).getClassName()); >> + >> + JavaClass jc = >> Repository.lookupClass(getObjectType(o).getClassName()); >> Field[] fields = jc.getFields(); >> Field f = null; >> for (Field field : fields) { >> @@ -997,7 +1007,7 @@ public final class Pass3aVerifier extend >> public void visitPUTSTATIC(PUTSTATIC o){ >> try { >> String field_name = o.getFieldName(cpg); >> - JavaClass jc = >> Repository.lookupClass(o.getClassType(cpg).getClassName()); >> + JavaClass jc = >> Repository.lookupClass(getObjectType(o).getClassName()); >> Field[] fields = jc.getFields(); >> Field f = null; >> for (Field field : fields) { >> @@ -1011,8 +1021,9 @@ public final class Pass3aVerifier extend >> } >> >> if (f.isFinal()){ >> - if >> (!(myOwner.getClassName().equals(o.getClassType(cpg).getClassName()))){ >> - constraintViolated(o, "Referenced field '"+f+"' is >> final and must therefore be declared in the current class >> '"+myOwner.getClassName()+"' which is not the case: it is declared in >> '"+o.getClassType(cpg).getClassName()+"'."); >> + if >> (!(myOwner.getClassName().equals(getObjectType(o).getClassName()))){ >> + constraintViolated(o, "Referenced field '"+f+"' is >> final and must therefore be declared in the current class >> '"+myOwner.getClassName() >> + +"' which is not the case: it is declared in >> '"+o.getReferenceType(cpg)+"'."); >> } >> } >> >> @@ -1037,7 +1048,7 @@ public final class Pass3aVerifier extend >> public void visitGETSTATIC(GETSTATIC o){ >> try { >> String field_name = o.getFieldName(cpg); >> - JavaClass jc = >> Repository.lookupClass(o.getClassType(cpg).getClassName()); >> + JavaClass jc = >> Repository.lookupClass(getObjectType(o).getClassName()); >> Field[] fields = jc.getFields(); >> Field f = null; >> for (Field field : fields) { >> >> Modified: >> commons/proper/bcel/trunk/src/main/java/org/apache/commons/bcel6/verifier/structurals/InstConstraintVisitor.java >> URL: >> http://svn.apache.org/viewvc/commons/proper/bcel/trunk/src/main/java/org/apache/commons/bcel6/verifier/structurals/InstConstraintVisitor.java?rev=1695425&r1=1695424&r2=1695425&view=diff >> ============================================================================== >> --- >> commons/proper/bcel/trunk/src/main/java/org/apache/commons/bcel6/verifier/structurals/InstConstraintVisitor.java >> (original) >> +++ >> commons/proper/bcel/trunk/src/main/java/org/apache/commons/bcel6/verifier/structurals/InstConstraintVisitor.java >> Wed Aug 12 06:32:41 2015 >> @@ -1208,6 +1208,15 @@ public class InstConstraintVisitor exten >> } >> } >> >> + private ObjectType getObjectType(FieldInstruction o) { >> + ReferenceType rt = o.getReferenceType(cpg); >> + if(rt instanceof ObjectType) { >> + return (ObjectType)rt; >> + } >> + constraintViolated(o, "expecting ObjectType but got "+rt); >> + return null; >> + } >> + >> /** >> * Ensures the specific preconditions of the said instruction. >> */ >> @@ -1221,7 +1230,7 @@ public class InstConstraintVisitor exten >> >> String field_name = o.getFieldName(cpg); >> >> - JavaClass jc = >> Repository.lookupClass(o.getClassType(cpg).getClassName()); >> + JavaClass jc = >> Repository.lookupClass(getObjectType(o).getClassName()); >> Field[] fields = jc.getFields(); >> Field f = null; >> for (Field field : fields) { >> @@ -1263,7 +1272,7 @@ public class InstConstraintVisitor exten >> } >> >> if (f.isProtected()){ >> - ObjectType classtype = o.getClassType(cpg); >> + ObjectType classtype = getObjectType(o); >> ObjectType curr = ObjectType.getInstance(mg.getClassName()); >> >> if ( classtype.equals(curr) || >> @@ -2632,7 +2641,7 @@ public class InstConstraintVisitor exten >> >> String field_name = o.getFieldName(cpg); >> >> - JavaClass jc = >> Repository.lookupClass(o.getClassType(cpg).getClassName()); >> + JavaClass jc = >> Repository.lookupClass(getObjectType(o).getClassName()); >> Field[] fields = jc.getFields(); >> Field f = null; >> for (Field field : fields) { >> @@ -2684,7 +2693,7 @@ public class InstConstraintVisitor exten >> } >> >> if (f.isProtected()){ >> - ObjectType classtype = o.getClassType(cpg); >> + ObjectType classtype = getObjectType(o); >> ObjectType curr = ObjectType.getInstance(mg.getClassName()); >> >> if ( classtype.equals(curr) || >> @@ -2722,7 +2731,7 @@ public class InstConstraintVisitor exten >> public void visitPUTSTATIC(PUTSTATIC o){ >> try { >> String field_name = o.getFieldName(cpg); >> - JavaClass jc = >> Repository.lookupClass(o.getClassType(cpg).getClassName()); >> + JavaClass jc = >> Repository.lookupClass(getObjectType(o).getClassName()); >> Field[] fields = jc.getFields(); >> Field f = null; >> for (Field field : fields) { >> >> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
