You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Gary Gregory <ga...@gmail.com> on 2015/08/15 04:53:02 UTC

Re: BCEL design goals? (was RE: [jira] [Commented] (BCEL-233) The access_flags field in AccessFlags class should be final)

On Fri, Aug 14, 2015 at 3:55 PM, Mark Roberts <ma...@cs.washington.edu>
wrote:

> I hope this doesn't come across as too strident, but working towards a
> resolution of this particular issue has got me thinking about how I use
> BCEL - and this seems like a good time/place to clarify my understanding of
> BCEL's purpose.
> Simply stated, it is my belief that a client should be able to read and/or
> write any member, field, flag, or what have you in a .class file.
>

Sounds reasonable. You should be able to fiddle with anything in a .class
file. Perhaps even shooting yourself in the foot ;-)

Gary

>
> Is that assuming too much?  Are there any intentional restrictions?
>
> Thanks,
> Mark
>
>
> -----Original Message-----
> From: Sebb (JIRA) [mailto:jira@apache.org]
> Sent: Friday, August 14, 2015 3:30 PM
> To: markro@cs.washington.edu
> Subject: [jira] [Commented] (BCEL-233) The access_flags field in
> AccessFlags class should be final
>
>
>     [
> https://issues.apache.org/jira/browse/BCEL-233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14697872#comment-14697872
> ]
>
> Sebb commented on BCEL-233:
> ---------------------------
>
> OK, have you got any sample code that could be used as test cases?
>
> > The access_flags field in AccessFlags class should be final
> > -----------------------------------------------------------
> >
> >                 Key: BCEL-233
> >                 URL: https://issues.apache.org/jira/browse/BCEL-233
> >             Project: Commons BCEL
> >          Issue Type: Improvement
> >            Reporter: Sebb
> >             Fix For: 6.0
> >
> >
> > The access_flags field in the AccessFlags class should be final.
> > It is currently not set by any other classes outside their constructors.
> > The public setters - e.g. isPrivate(boolean) - are not used and should
> be deleted. [Apart from that, their names are really confusing.]
>
>
>
> --
> This message was sent by Atlassian JIRA
> (v6.3.4#6332)
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

Re: BCEL design goals? (was RE: [jira] [Commented] (BCEL-233) The access_flags field in AccessFlags class should be final)

Posted by sebb <se...@gmail.com>.
OK, I see. I guess I need to revert all those changes.

I had hoped to make as much as possible immutable, and hide all
mutable fields, but this looks to be impossible with the current
design.

I'm beginning to wonder if we should try to revert to being
binary-compatible with the previous version.

Although the Clirr report showed some errors, Clirr does not
distinguish between binary and source compatibility.
As it happens, adding methods to an Interface is binary-compatible [1]
Of course it will break source compatibility if the Visitor interface
is implemented directly; though not if the EmptyVisitor class is
extended instead.

The downside of remaining binary compatible is that some useful fixes
cannot be implemented.

Once I have reverted the setters/non-private mutable fields we can see
what breaking changes remain.
It will then be a trade-off to see whether it is worth reverting the
other fixes to restore compatibilty, or whether we really do need to
break the API in places.

[1] https://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html#jls-13.5.3-100

On 17 August 2015 at 16:37, Mark Roberts <ma...@cs.washington.edu> wrote:
> That's a little tricky to answer.  As one example, look at generic/LocalVariableInstruction.  Neither the index or opcode is final.  A user needs to be able to insert new local variables and hence they need to be able to modify the local variable offset (index).  And 'beneath the covers' BCEL will modify the opcode if the index changes from one side of the single byte opcode to the other.  E.g., it will change an 'aload_<n>' to an 'aload'; thus changing both the opcode and the length of the instruction.
>
> As a general rule, you might try to say you can only replace an instruction rather than modify it.  Personally, I think that is too restrictive.
>
>
> -----Original Message-----
> From: sebb [mailto:sebbaz@gmail.com]
> Sent: Sunday, August 16, 2015 5:17 AM
> To: Commons Developers List
> Subject: Re: BCEL design goals? (was RE: [jira] [Commented] (BCEL-233) The access_flags field in AccessFlags class should be final)
>
> I think it's still an error to change certain aspects of an Instruction class.
> For example, the opcode and length are fixed for a particular instruction type.
>
>
>
> ---------------------------------------------------------------------
> 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


RE: BCEL design goals? (was RE: [jira] [Commented] (BCEL-233) The access_flags field in AccessFlags class should be final)

Posted by Mark Roberts <ma...@cs.washington.edu>.
That's a little tricky to answer.  As one example, look at generic/LocalVariableInstruction.  Neither the index or opcode is final.  A user needs to be able to insert new local variables and hence they need to be able to modify the local variable offset (index).  And 'beneath the covers' BCEL will modify the opcode if the index changes from one side of the single byte opcode to the other.  E.g., it will change an 'aload_<n>' to an 'aload'; thus changing both the opcode and the length of the instruction.

As a general rule, you might try to say you can only replace an instruction rather than modify it.  Personally, I think that is too restrictive.


-----Original Message-----
From: sebb [mailto:sebbaz@gmail.com] 
Sent: Sunday, August 16, 2015 5:17 AM
To: Commons Developers List
Subject: Re: BCEL design goals? (was RE: [jira] [Commented] (BCEL-233) The access_flags field in AccessFlags class should be final)

I think it's still an error to change certain aspects of an Instruction class.
For example, the opcode and length are fixed for a particular instruction type.



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


Re: BCEL design goals? (was RE: [jira] [Commented] (BCEL-233) The access_flags field in AccessFlags class should be final)

Posted by sebb <se...@gmail.com>.
On 16 August 2015 at 13:00, sebb <se...@gmail.com> wrote:
> On 15 August 2015 at 03:53, Gary Gregory <ga...@gmail.com> wrote:
>> On Fri, Aug 14, 2015 at 3:55 PM, Mark Roberts <ma...@cs.washington.edu>
>> wrote:
>>
>>> I hope this doesn't come across as too strident, but working towards a
>>> resolution of this particular issue has got me thinking about how I use
>>> BCEL - and this seems like a good time/place to clarify my understanding of
>>> BCEL's purpose.
>>> Simply stated, it is my belief that a client should be able to read and/or
>>> write any member, field, flag, or what have you in a .class file.
>>>
>>
>> Sounds reasonable. You should be able to fiddle with anything in a .class
>> file. Perhaps even shooting yourself in the foot ;-)
>
> Sorry, I overlooked this important use case.
> I had assumed that the BCEL classes would be used to represent an
> existing class file or be used to create a new classfile.
> In such cases they could be made immutable.
>
> However, that does not allow for easily modifying existing classes.
> [It would still be possible, but would require creating new instances]
>
> Now I think there are still some classes which should not be mutable
> once created.
> For example, Instruction is the super-class for IADD; it does not make
> sense to modify an IADD once created.
> I'm not sure about other Instruction classes either - although BRANCH
> instructions make have multiple distinct instances, should they be
> mutable?
> If so, then I suspect the InstructionList should not be allowed to
> share any mutable Instructions.
> Otherwise updating one GOTO may accidentally update other unrelated
> GOTOs that happen to have the same offset

Ignore that - Branches refer to Instruction Handles not relative
offsets as I had assumed.

> Maybe the solution to this is to make the classfile.* classes fully
> mutable, but make the generic.* classes immutable?

I think it's still an error to change certain aspects of an Instruction class.
For example, the opcode and length are fixed for a particular instruction type.

>> Gary
>>
>>>
>>> Is that assuming too much?  Are there any intentional restrictions?
>>>
>>> Thanks,
>>> Mark
>>>
>>>
>>> -----Original Message-----
>>> From: Sebb (JIRA) [mailto:jira@apache.org]
>>> Sent: Friday, August 14, 2015 3:30 PM
>>> To: markro@cs.washington.edu
>>> Subject: [jira] [Commented] (BCEL-233) The access_flags field in
>>> AccessFlags class should be final
>>>
>>>
>>>     [
>>> https://issues.apache.org/jira/browse/BCEL-233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14697872#comment-14697872
>>> ]
>>>
>>> Sebb commented on BCEL-233:
>>> ---------------------------
>>>
>>> OK, have you got any sample code that could be used as test cases?
>>>
>>> > The access_flags field in AccessFlags class should be final
>>> > -----------------------------------------------------------
>>> >
>>> >                 Key: BCEL-233
>>> >                 URL: https://issues.apache.org/jira/browse/BCEL-233
>>> >             Project: Commons BCEL
>>> >          Issue Type: Improvement
>>> >            Reporter: Sebb
>>> >             Fix For: 6.0
>>> >
>>> >
>>> > The access_flags field in the AccessFlags class should be final.
>>> > It is currently not set by any other classes outside their constructors.
>>> > The public setters - e.g. isPrivate(boolean) - are not used and should
>>> be deleted. [Apart from that, their names are really confusing.]
>>>
>>>
>>>
>>> --
>>> This message was sent by Atlassian JIRA
>>> (v6.3.4#6332)
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>>
>>
>>
>> --
>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>> Java Persistence with Hibernate, Second Edition
>> <http://www.manning.com/bauer3/>
>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>> Spring Batch in Action <http://www.manning.com/templier/>
>> Blog: http://garygregory.wordpress.com
>> Home: http://garygregory.com/
>> Tweet! http://twitter.com/GaryGregory

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


Re: BCEL design goals? (was RE: [jira] [Commented] (BCEL-233) The access_flags field in AccessFlags class should be final)

Posted by sebb <se...@gmail.com>.
On 15 August 2015 at 03:53, Gary Gregory <ga...@gmail.com> wrote:
> On Fri, Aug 14, 2015 at 3:55 PM, Mark Roberts <ma...@cs.washington.edu>
> wrote:
>
>> I hope this doesn't come across as too strident, but working towards a
>> resolution of this particular issue has got me thinking about how I use
>> BCEL - and this seems like a good time/place to clarify my understanding of
>> BCEL's purpose.
>> Simply stated, it is my belief that a client should be able to read and/or
>> write any member, field, flag, or what have you in a .class file.
>>
>
> Sounds reasonable. You should be able to fiddle with anything in a .class
> file. Perhaps even shooting yourself in the foot ;-)

Sorry, I overlooked this important use case.
I had assumed that the BCEL classes would be used to represent an
existing class file or be used to create a new classfile.
In such cases they could be made immutable.

However, that does not allow for easily modifying existing classes.
[It would still be possible, but would require creating new instances]

Now I think there are still some classes which should not be mutable
once created.
For example, Instruction is the super-class for IADD; it does not make
sense to modify an IADD once created.
I'm not sure about other Instruction classes either - although BRANCH
instructions make have multiple distinct instances, should they be
mutable?
If so, then I suspect the InstructionList should not be allowed to
share any mutable Instructions.
Otherwise updating one GOTO may accidentally update other unrelated
GOTOs that happen to have the same offset

Maybe the solution to this is to make the classfile.* classes fully
mutable, but make the generic.* classes immutable?

> Gary
>
>>
>> Is that assuming too much?  Are there any intentional restrictions?
>>
>> Thanks,
>> Mark
>>
>>
>> -----Original Message-----
>> From: Sebb (JIRA) [mailto:jira@apache.org]
>> Sent: Friday, August 14, 2015 3:30 PM
>> To: markro@cs.washington.edu
>> Subject: [jira] [Commented] (BCEL-233) The access_flags field in
>> AccessFlags class should be final
>>
>>
>>     [
>> https://issues.apache.org/jira/browse/BCEL-233?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14697872#comment-14697872
>> ]
>>
>> Sebb commented on BCEL-233:
>> ---------------------------
>>
>> OK, have you got any sample code that could be used as test cases?
>>
>> > The access_flags field in AccessFlags class should be final
>> > -----------------------------------------------------------
>> >
>> >                 Key: BCEL-233
>> >                 URL: https://issues.apache.org/jira/browse/BCEL-233
>> >             Project: Commons BCEL
>> >          Issue Type: Improvement
>> >            Reporter: Sebb
>> >             Fix For: 6.0
>> >
>> >
>> > The access_flags field in the AccessFlags class should be final.
>> > It is currently not set by any other classes outside their constructors.
>> > The public setters - e.g. isPrivate(boolean) - are not used and should
>> be deleted. [Apart from that, their names are really confusing.]
>>
>>
>>
>> --
>> This message was sent by Atlassian JIRA
>> (v6.3.4#6332)
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> Java Persistence with Hibernate, Second Edition
> <http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory

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