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

Fixes for the previously merged TryFrom #10

Closed
wants to merge 6 commits into from
Closed

Fixes for the previously merged TryFrom #10

wants to merge 6 commits into from

Conversation

regexident
Copy link
Contributor

Turns out generating shim code via build.rs and relying on being correct without compiling/running any of it during testing is a bad idea. Code that does not get compiled won't fail tests either. Embarrassing.

Fixes:

  • I used #[cfg(feature = “try_from”)] instead #[cfg(feature = “try-from”)] causing my code to not even get generated. (#fdda4b1)
  • Had a wrong error type in one function that did not get caught. (#07319d7)
  • Had a wrong implementation in one function that did not get caught. (#e3d0328)

Changes:

  • Switched errors from core::num::TryFromIntError to cast::Error. (#35a3fbd)
  • Removed impls of TryFrom for usize/isize to mirror ‘stdlib’. (#67a17b1)
  • Refactored unit tests. Let’s actually run them this time, shall we? (#b134161)

As part of the unit test refactoring we are now also:

  • Testing From and TryFrom for every variant of cast::From, where applicable.
  • Splitting asserts into smaller test functions making it easier to see what's wrong.

We now have 418 tests for num-traits and 477 tests for cast::From/From/TryFrom.

@japaric
Copy link
Owner

japaric commented Apr 2, 2018

Thanks @regexident

I think you may need to install the rustfmt-preview in the Travis builder for the buld script to work, though? Let's see what bors says.

bors r+

bors bot added a commit that referenced this pull request Apr 2, 2018
10: Fixes for the previously merged `TryFrom` r=japaric a=regexident

Turns out generating shim code via `build.rs` and relying on being correct without compiling/running any of it during testing is a bad idea. Code that does not get compiled won't fail tests either. Embarrassing.

Fixes:
- I used `#[cfg(feature = “try_from”)]` instead `#[cfg(feature = “try-from”)]` causing my code to not even get generated. (#fdda4b1)
- Had a wrong error type in one function that did not get caught. (#07319d7)
- Had a wrong implementation in one function that did not get caught. (#e3d0328)

Changes:
- Switched errors from `core::num::TryFromIntError` to `cast::Error`. (#35a3fbd)
- Removed impls of `TryFrom` for `usize`/`isize` to mirror ‘stdlib’. (#67a17b1)
- Refactored unit tests. Let’s actually run them this time, shall we? (#b134161)

As part of the unit test refactoring we are now also:
- Testing `From` and `TryFrom` for every variant of `cast::From`, where applicable.
- Splitting asserts into smaller test functions making it easier to see what's wrong.

We now have **418 tests** for `num-traits` and **477 tests** for `cast::From`/`From`/`TryFrom`.
@bors
Copy link

bors bot commented Apr 2, 2018

Build failed

@japaric
Copy link
Owner

japaric commented Apr 2, 2018

error[E0599]: no function or associated item named try_from found for type fpa::Q<i8, typenum::uint::UInt<typenum::uint::UTerm, typenum::bit::B1>> in the current scope
--> /home/travis/build/japaric/fpa/target/debug/build/fpa-cb4c4ea6a8ca90bd/out/cross.rs:1:4359

@regexident
Copy link
Contributor Author

Could we try this one again, @japaric?
(Is there a way for me to trigger bors, or is it up to you, exclusively?)

@regexident
Copy link
Contributor Author

Any chance to have this re-tested and merged, @japaric?

@regexident regexident closed this by deleting the head repository Aug 3, 2024
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.

2 participants