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

Use several nice newish Rust features #1024

Merged
merged 6 commits into from Jun 29, 2018

Conversation

Projects
None yet
3 participants
@bradjc
Copy link
Contributor

bradjc commented Jun 23, 2018

Pull Request Overview

As a push towards "Rust 2018", Rust is advertising several new features that are actually quite nice for Tock. This site: https://rust-lang-nursery.github.io/edition-guide/2018/index.html explains them.

What we really benefit from is that before we had to do:

impl<'a, A: Alarm + 'a> MyStruct<'a, A>

Now rust will just figure all that out, and we only need:

impl<A: Alarm> MyStruct<'a, A>

And the simple case is clean now. Before:

impl<'a> MyOtherStruct<'a>

now:

impl MyOtherStruct<'a>

This is I think a lot closer to what someone new to rust would try to write.

Testing Strategy

This pull request was tested by compiling.

TODO or Help Wanted

n/a

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make formatall.

@bradjc bradjc force-pushed the struct-lifetimes branch from a072702 to fd61bb1 Jun 24, 2018

@bradjc bradjc added the blocked label Jun 25, 2018

@bradjc

This comment has been minimized.

Copy link
Contributor

bradjc commented Jun 25, 2018

Hopefully the docs will build on the new nightly.

@@ -147,7 +147,7 @@ impl<'a, A: time::Alarm> capsules::net::icmpv6::icmpv6_send::ICMP6SendClient
}
}

impl<'a, A: time::Alarm + 'a> LowpanICMPTest<'a, A> {
impl<'a, A: time::Alarm> LowpanICMPTest<'a, A> {

This comment has been minimized.

@brghena

brghena Jun 25, 2018

Contributor

Why can't we remove the first 'a here? (and numerous other places such as ipv6_lowpan_test.rs, sixlowpan_dummy.rs, udp_lowpan_test.rs, and traitobj_list.rs)

This comment has been minimized.

@bradjc

bradjc Jun 25, 2018

Contributor

I think we can. Commits welcome :) (It's a test file so my motivation was low).

This comment has been minimized.

@ppannuto

ppannuto Jun 27, 2018

Member

First try doesn't "just work":

   Compiling imix v0.1.0 (file:///Volumes/code/helena-project/tock/boards/imix)
error[E0106]: missing lifetime specifier
   --> src/icmp_lowpan_test.rs:150:22
    |
150 | impl<A: time::Alarm> LowpanICMPTest<A> {
    |                      ^^^^^^^^^^^^^^^^^ expected lifetime parameter

error[E0106]: missing lifetime specifier
   --> src/icmp_lowpan_test.rs:157:10
    |
157 |     ) -> LowpanICMPTest<A> {
    |          ^^^^^^^^^^^^^^^^^ expected lifetime parameter
    |
    = help: this function's return type contains a borrowed value, but the signature does not say which one of `icmp_sender`'s 2 lifetimes it is borrowed from

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0106`.

Messing around a bit I couldn't puzzle out any other thing that would make the compiler happy, but I didn't try too hard

@bradjc bradjc force-pushed the struct-lifetimes branch from fd61bb1 to 7a360c0 Jun 26, 2018

@bradjc bradjc removed the blocked label Jun 26, 2018

@bradjc bradjc force-pushed the struct-lifetimes branch from 7a360c0 to a5e38b0 Jun 27, 2018

bradjc added some commits Jun 23, 2018

use infer_outlives_requirements feature
rust-lang/rust#44493

This removes the need for extra lifetimes in structs.
add `in_band_lifetimes` feature
Remove a bunch of `impl<'a` yay!

@ppannuto ppannuto force-pushed the struct-lifetimes branch from a5e38b0 to 138683e Jun 27, 2018

imix: remove one more 'a
Mostly just learning how this works, can certainly apply more places.

@ppannuto ppannuto added the P-Upkeep label Jun 28, 2018

@bradjc

This comment has been minimized.

Copy link
Contributor

bradjc commented Jun 29, 2018

Thoughts?

@ppannuto
Copy link
Member

ppannuto left a comment

I'm sure there's more places we could keep making more of these fixes, but more better code merged now is 👍 for me

@ppannuto ppannuto merged commit 5b410e3 into master Jun 29, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details

@ppannuto ppannuto deleted the struct-lifetimes branch Jun 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment