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 error types #150

Merged
merged 3 commits into from
Nov 3, 2019
Merged

Fix error types #150

merged 3 commits into from
Nov 3, 2019

Conversation

Disasm
Copy link
Member

@Disasm Disasm commented Aug 20, 2019

core::convert::Infallible was stabilized in 1.34.0. This PR replaces Void and () error types with Infallible.

Note that this is a breaking change.

@rust-highfive
Copy link

r? @therealprof

(rust_highfive has picked a reviewer for you, use r? to override)

@thejpster
Copy link
Contributor

What's our stated MSRV?

@ryankurte
Copy link
Contributor

@thejpster huh, looks like we haven't documented one in this repo yet?

a few thoughts without particular direction:

  • i wonder what the migration path for the void package is, it looks like there was a suggestion they move to '!' some time ago
  • it wouldn't let void be removed completely, but, might be possible instead to add a second shim with the Infallible type to digital::v2_convert so older implementations would continue to work (until void or aliases change anyway)
  • Infallible looks to be a temporary measure prior to stabilisation of !, does it make sense to switch now or later?

@Disasm
Copy link
Member Author

Disasm commented Aug 21, 2019

@ryankurte As for me, Infallible is a nice replacement for Void: it's pretty the same and doesn't require an additional crate.
Anyway, my major concern was () error type in v2_compat: it doesn't provide a hint to the compiler that error condition never happens.

@thejpster
Copy link
Contributor

Given our policy:

The MSRV shall always be strictly smaller than the latest available minor stable release. For example, if the latest stable Rust version was 1.32.1 a new minor release of the cortex-m crate can't bump the MSRV higher than 1.31.

And that this only requires 1.34, I'm happy to merge this. Infallible will alias ! when it stabilises in 1.40, so we can take a PR to move to ! after 1.41 is out.

@thejpster
Copy link
Contributor

@rust-embedded/hal any objections to merging?

@therealprof
Copy link
Contributor

Let's do this.

@therealprof
Copy link
Contributor

bors r+

bors bot added a commit that referenced this pull request Nov 2, 2019
150: Fix error types r=therealprof a=Disasm

`core::convert::Infallible` was stabilized in 1.34.0. This PR replaces `Void` and `()` error types with `Infallible`.

Note that this is a breaking change.

Co-authored-by: Vadim Kaushan <admin@disasm.info>
@bors
Copy link
Contributor

bors bot commented Nov 2, 2019

Build failed

@therealprof
Copy link
Contributor

Ah, the joy of requiring a funtioning nightly build.

@Disasm
Copy link
Member Author

Disasm commented Nov 3, 2019

Cargo.lock is quite outdated, retrying
bors r=therealprof

bors bot added a commit that referenced this pull request Nov 3, 2019
150: Fix error types r=therealprof a=Disasm

`core::convert::Infallible` was stabilized in 1.34.0. This PR replaces `Void` and `()` error types with `Infallible`.

Note that this is a breaking change.

Co-authored-by: Vadim Kaushan <admin@disasm.info>
@bors
Copy link
Contributor

bors bot commented Nov 3, 2019

Build succeeded

@bors bors bot merged commit c579281 into rust-embedded:master Nov 3, 2019
@Disasm Disasm deleted the fix-error-type branch November 3, 2019 05:14
@ryankurte
Copy link
Contributor

PR void to assist with interoperability while we're migrating things: reem/rust-void#20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Review is incomplete T-hal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants