Implement assemble func prototype #72
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AgwaB
changed the title
|
Moved |
AgwaB
force-pushed
the
feature/vm/asm
branch
3 times, most recently
from
7201216 to
c5b83a0
Compare
|
I think |
junbeomlee
reviewed
Jan 4, 2019
|
|
||
| if !ok { | ||
| return nil, errors.New("Invalid byteCode") | ||
| } |
There was a problem hiding this comment.
I think it's better to predefine
var ErrInvalidOpcode = errors.New("Invalid byteCode") and return ErrInvalidOpcode
junbeomlee
reviewed
Jan 4, 2019
| _, err := assemble(rawByteCode) | ||
| if err == nil { | ||
| t.Error("The desired error was not found ") | ||
| } |
There was a problem hiding this comment.
It would be better to compare like this
if err != ErrInvalidOpcode{
t.Error("The desired error was not found ")
}
junbeomlee
reviewed
Jan 4, 2019
| asm.jump(2) | ||
| asm.jump(4) | ||
| asm.jump(10) // Error occur | ||
| } |
There was a problem hiding this comment.
It is important to distinguish between cases where err occurs and cases that do not occur.
In cases where err does not occur, it is necessary to check whether asm's pc value is correctly jumped.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
details:
We need to discuss about #60
But when we implement opCodes( #61 ), it needs
asm.code( it is analyzed
rawByteCodebut the analysis is focused on cost maybe. )I think it is better to implement the analysis properly after it is fully discussed!! so i implement prototype of
analysis...