commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1695425 - in /commons/proper/bcel/trunk/src/main/java/org/apache/commons/bcel6: generic/FieldOrMethod.java verifier/statics/Pass3aVerifier.java verifier/structurals/InstConstraintVisitor.java
Date Thu, 10 Sep 2015 22:26:52 GMT
Thanks, fix looks good.

On 9 September 2015 at 14:13, Chas Honton <chas@honton.org> wrote:
> Will work in this evening. (GMT-8).
>
> Chas
>
>> On Sep 9, 2015, at 2:07 AM, sebb <sebbaz@gmail.com> wrote:
>>
>> PING
>>
>> I am -1 on the commit as it stands; please revert or fix
>>
>>> On 28 August 2015 at 01:43, sebb <sebbaz@gmail.com> wrote:
>>>> On 12 August 2015 at 07:32,  <chas@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-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message