RFC: Replace all uses of TakeCell with OptionalCell

[hudson-ayers]

Pull Request Overview

This pull request makes some slight modifications to the OptionalCell interface, and then replaces all uses of TakeCell with OptionalCell. I have had a couple students in the past few months ask me why Tock included both when TakeCell seems to just offer a subset of the functionality of OptionalCell, and after looking into it I found that was in fact the case. I don't think it makes sense to use 2 nearly identical types when there doesn't seem to be a compelling reason for doing so.

As part of doing so, this PR removes some functions that artificially required T: Copy within OptionalCell, by using the approach that previously existed in take_cell.rs to not do so. This makes it easier to use OptionalCell for a broader range of types.

I do not think this would have been possible when TakeCell was first created, Cell has come a long way since then.

There are a few downsides I see in this PR, which is why it is an RFC:

1. Instead of #[feature(const_mut_refs)] only applying to the tock-cellscrate, it now applies to all crates that were previously using TakeCell::empty(). This isn't really different in terms of how the code compiles, but means that it might be easier for developers to accidentally utilize this feature in chips/ or capsules/. However I am not actually that worried by this, as the new peripheral instantiation approach has removed any fundamental need for const peripheral constructors anyway.

2. I added a put() method to OptionalCell that does the same thing as insert() because TakeCell and OptionalCell used different names for this operation (and my IDE's structural-search-and-replace was not working as I expected).

3. Will probably make the tock 2.0 branch harder to merge in unless I apply this diff to it as well.

If anyone is curious, I made most of the rote changes using the following bash script:

#!/bin/bash

# first replace all uses (not imports)
grep -rli "TakeCell<'.*,.*>" * | xargs -i@ sed -i "s/TakeCell<'\(.*\),\(.*\)>/OptionalCell<\&'\1 mut \2>/g" @

#Fix standalone use statements, comments referencing TakeCell
grep -rli "TakeCell" * | xargs -i@ sed -i "s/TakeCell/OptionalCell/g" @

#Previous line will lead to double imports within a line
grep -rli "use .*OptionalCell.*OptionalCell" * | xargs -i@ sed -i "s/use \(.*\)OptionalCell\(.*\)OptionalCell/use \1OptionalCell\2/g" @

#and double imports across lines
grep -rli "use .*cells::OptionalCell;" * | xargs -i@ gawk -i inplace '$0=="use kernel::common::cells::OptionalCell;"{c[$0]++} c[$0]<2; ' @

#clean up commas and whitespace and such
make format

rm ~/tock/libraries/tock-cells/src/take_cell.rs

Testing Strategy

This pull request was tested by compiling.

TODO or Help Wanted

N/A

Documentation Updated

• Updated \doc/Mutable_References.md to remove all TakeCell discussion. Maybe we should expand the OptionalCell discussion though?

Formatting

• Ran make prepush.

[bradjc]

+1 from me, seems good to deprecate a type if Rust has evolved enough to no longer need it.

[hudson-ayers]

@alistair23 This fails the QEMU CI, and I am trying to figure out why.

When I locally run make qemu for the hifive, I get
"HiFive1 Initialization Complete"

..but I do not get the second debug statement ("Entering main loop").

There is the following note in hifive1's main.rs:

    // Need two debug!() calls to actually test with QEMU. QEMU seems to have
    // a much larger UART TX buffer (or it transmits faster).
    debug!("HiFive1 initialization complete.");
    debug!("Entering main loop.");

Do you know of a reason that the first debug might work, but the second does not? I do not see any issues running on Imix after a couple quick tests.

[hudson-ayers]

actually, I am able to reproduce this on Imix...something in the console seems to have broken

[hudson-ayers]

Ok, I think I jumped the gun here. The way map() works is actually different for OptionalCell and TakeCell, as you can call map() on an OptionalCell within a closure passed to map() on that same OptionalCell, as a result of the Copy bound. There are places in the Tock kernel that rely on this behavior, and changing OptionalCell to the TakeCell semantics (where calling map() the second time does nothing) breaks that code. Sorry for the noise.