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

Fix compilation with latest rustc #36

Closed
wants to merge 1 commit into from
Closed

Conversation

mre
Copy link

@mre mre commented Aug 1, 2017

Some adjustments to make this crate work with the Rust 1.19.

Major changes:

  • Some functionality (bitflags, num) has been removed from the standard library. Replaced the imports with the respective crates.
  • Implement Clone for custom types
  • Minor syntax changes in Rustlang
  • Formatting using the latest rustfmt

Currently some unit tests are failing. This is due to expressions like compare(&mut machine, !128); in machine.rs. Before this was compare(&mut machine, -128); and I don't know what this means, really. Thought the - was negation, but seems like I'm mistaken. In any way, the second parameter is a usize, so does the - make sense here?

@mre
Copy link
Author

mre commented Aug 2, 2017

Update: Replaced compare(&mut machine, -128); with compare(&mut machine, -128i8 as u8); thanks to the help on the rust irc channel. Still the unit tests fail.
Namely the failing tests are:

  • machine::tests::add_with_carry_test
  • machine::tests::compare_with_a_register_test
  • machine::tests::compare_with_x_register_test
  • machine::tests::compare_with_y_register_test
  • machine::tests::decrement_memory_test
  • machine::tests::subtract_with_carry_test

@mre
Copy link
Author

mre commented Aug 2, 2017

Just noticed that all tests except for one will pass when I run the tests like this:

cargo test --release

The reason is that overflow checking is disabled then. Was this the intended behavior?

The only missing test failing in release mode is this:

thread 'machine::tests::decrement_memory_test' panicked 
at 'index out of bounds: the len is 2 but the index is 41394'

@mre
Copy link
Author

mre commented Aug 3, 2017

Okay, I've fixed the remaining bug.
I've switched the byte slice for the memory to a vector (see here). The problem was that the initial allocation did not work. It was not allocating MEMORY_SIZE elements.

For some reason, the vec! syntax does not work. I've switched to this
.

Now all tests pass in release mode.

@mre
Copy link
Author

mre commented Aug 8, 2017

All tests pass in debug now, too.
Used the explicit wrapping_add() and wrapping_sub() methods to allow overflow.
Alternatively I could have used std::num::Wrapping to achieve the same, but I found it more verbose.

@mre
Copy link
Author

mre commented Aug 11, 2022

Just in case anyone is looking for a follow-up project, I forked this repo a while ago and a small group of people maintains it at https://github.com/mre/mos6502, so I think we can close this PR. Thanks for your initial work on the project. ❤️

@mre mre closed this Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant