Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use InstructionHandle as target instead of Instruction #2

Closed
wants to merge 2 commits into from
Closed

Use InstructionHandle as target instead of Instruction #2

wants to merge 2 commits into from

Conversation

chonton
Copy link
Member

@chonton chonton commented Aug 25, 2015

The programming model for BCEL instructions is confused. Although not explicitly documented, the InstructionList is read/written in the 'byte code' model and manipulated in the 'generic' (or perhaps more accurately 'symbolic') model. The program should edit using InstructionHandles rather than Instruction; except that currently, setTarget uses an Instruction, not an InstructionHandle.

Also, there is an attempt to reuse Instructions and InstructionHandles. I'm pretty sure this is mostly a bad idea. An InstructionHandle should exist only once in an InstructionList and should only belong to a single InstructionList. An Immutable Instruction may be held by multiple InstructionHandles. The Immutable Instructions should probably be tagged with an interface, etc.

Since these changes could be considered radical, I created this pull request rather than directly committing to svn. I request that our beta users give this branch and report any major concerns.

thanks,
chas

@garydgregory
Copy link
Member

Does this break BC?

@chonton chonton closed this Nov 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants