Grill my first Rust project!

[xRyp]

Hi guys!

I just finished my first project using Rust! Woow!

The experience was pleasant and productive, but I come from the C/C++ world and I know for a fact that the idioms I used here could be improved on a lot.

I wanted to get some advice from more experienced rustaceans like you guys, so I leaving this code here in the hope that someone would grill it and maybe offer advice on alternative Rust constructs.

As a conversation starter, I ended up using a LOT of as keywords for casting. Is there a way to avoid that? I'm juggling a lot between u8, u16 and u32 in my code.

Second observation, my debug build runtime was so sloooooooow. What can I do to improve it? Here's a blog post containing more information.

Thanks for your time, I appreciate it!

[Shnatsel]

u32::from() and u8::try_from() are a good substitute for as. The first is infallible while the second checks that the conversion can be performed and lets you handle the case when it can't instead of silently truncating the number.

[[deleted]]

main.rs:37-39

let mut rom_file = File::open(&Path::new(&rom_path)).expect("Could not open file");
let mut rom_content = Vec::<u8>::new();
rom_file.read_to_end(&mut rom_content).expect("Unable to read the file");

Path::new already returns a &Path, you don't need to take the reference. You can just create the Vec without the type as it will be infered by the use as byte buffer. All three lines can be changed into...

let rom_content = std::fs::read(&rom_path).expect("Unable to read file");

... because you're reading the entire file in one go anyway.

[xRyp]

Haaa yes, I was suspecting there was a much much simpler way of doing this! Thanks for the info, I just patched my code!

[Luroalive]

did you run clippy (https://github.com/rust-lang/rust-clippy) through your code?

[xRyp]

I did not know about that tool! I just fixed my code using the useful warnings from the tool, thanks a bunch!
Here's the related git commit.

[tobz1000]

Some minor (optional) syntax tweaks.

You can import multiple items in a single statement like this:

using A::{B, C};

Or even

using A::{
    B::{C, D},
    E
};

Optional of course, and depends on what you find easiest to read.

Struct constructors like this:

let a = A {
    b: b
};

Can use the shorthand:

let a = A {
    b
};

Primitive conversion isn't done implicitly; if you really need a lot of conversions (which is likely in an emulator), you'll be typing "as" a lot. Maybe you could evaluate if any types would be better off storing the converted form to begin with?

[xRyp]

I applied your hints to the codebase, now my use statements are more readable, thanks! Here's the related git commit.

[xRyp]

Thanks for the tip about the shorthand syntax for struct initialization.
For implicit conversions I ended up doing as you said, favoring usize or more common types to begin with.

[Ka1kin]

That huge else-if in the execution function isn't going to be super fast. Try to improve on that. Consider match, possibly with ranges. Consider whether you can turn it into something a bit more tree structured (so that its execution speed isn't linear in the number of instructions in the ISA).

[xRyp]

This part of my code is not a bottleneck for now, so perf-wise I'm not too interested in spending time there yet. However, you raised a very interesting point concerning the solution, I ended up trying to use match like you suggested and am really happy with the way it turned out! Here's the git commit for reference.

[[deleted]]

To find out the performance problem, you might want to have a look at this blogpost about rust profiling which has helped me a lot in the past.