Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Type opcodes table as const in @truffle/code-utils #5557

Merged
merged 2 commits into from
Sep 22, 2022
Merged

Conversation

gnidan
Copy link
Contributor

@gnidan gnidan commented Sep 21, 2022

I didn't like how this was needlessly clobbering type information, so this PR modifies @truffle/code-utils to preserve const-ness in its definition of the opcodes table.

Copy link
Contributor

@haltman-at haltman-at left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really clear to me that this is useful, but, uh, OK, I guess?


export = (op: number) => (codes[op] ? codes[op] : "INVALID");
export const parseOpcode = (op: number) =>
codes[op in codes ? (op as keyof OpcodeTable) : 0xfe];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using 0xfe as a default before the lookup, rather than "INVALID" after, is pretty confusing. I think it would be better to do this the old way (not necessarily the same code, but at least doing the defaulting after), or, failing that, there should at least be a comment to explain this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okee! Yeah I was on the fence. I'll switch it back.

@gnidan
Copy link
Contributor Author

gnidan commented Sep 21, 2022

Not really clear to me that this is useful, but, uh, OK, I guess?

As usual, this work is driven by some offhand hacking :)

Copy link
Contributor

@haltman-at haltman-at left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I guess we can do this?

@gnidan
Copy link
Contributor Author

gnidan commented Sep 21, 2022

Thanks!

@gnidan gnidan merged commit 86fc812 into develop Sep 22, 2022
@gnidan gnidan deleted the const-ops branch September 22, 2022 00:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants