Improved const evaluation for ZST checks.

[ErisianArchitect]

I was reading through the source code, and I noticed the usage of is_zst() as a condition in some if statements. Rust does not guarantee that const functions will be evaluated at compile time, so the compiler may not perform dead code elimination on these if statements. I removed the is_zst functions from type implementations, and instead added a generic parameter to TaggedLen functions and forced is_zst to be evaluated at compile time. This should guarantee dead code elimination in some cases.

Edit: Sorry, I forgot to mention, I also replaced the is_zst function with a const in some places. Maybe one or two. Yet again, for dead code elimination.

[jdm]

Really interesting! One concern I have is that nothing prevents using unrelated types for the TaggedLen methods that are now generic. Would it make more sense for TaggedLen to be generic and contain a PhantomData member to constrain it and reduce the need for manual type annotations?

[ErisianArchitect]

I thought about doing that with the PhantomData. There's no reason it couldn't be done.

[ErisianArchitect]

I moved T into TaggedLen using PhantomData, and also (hopefully) fixed the linux errors that I didn't catch because I'm on Windows.

Edit: Oh, I almost forgot. The reason I originally didn't use PhantomData was because it interfered with the derived Clone and Copy implementations, so I had to implement those manually.

[jdm]

Thanks! I think the result of these changes is much clearer.

[jdm]

Really not sure what to do about the mysterious failure in the miri build, unfortunately.

[ErisianArchitect]

Really not sure what to do about the mysterious failure in the miri build, unfortunately.

Yeah, I'm not either, but maybe removing those unnecessary unsafe blocks would be a good start. I thought about removing them myself, but I wasn't sure if there was something going on that was hidden by the macro, so I just left it.

[jdm]

I'll try that in #402.

[ErisianArchitect]

Well, it says something about outdated JSON and recommends using cargo clean. I don't think running cargo clean would work, anyway, because the CI is running a fresh version every time. Maybe try updating serde?

[jdm]

It should already use the latest version of Serde, since there's no Cargo.lock file pinning it.

[ErisianArchitect]

#!/usr/bin/bash

set -ex

# Clean out our target dir, which may have artifacts compiled by a version of
# rust different from the one we're about to download.
cargo clean

# Install and run the latest version of nightly where miri built successfully.
# Taken from: https://github.com/rust-lang/miri#running-miri-on-ci

MIRI_NIGHTLY=nightly-$(curl -s https://rust-lang.github.io/rustup-components-history/x86_64-unknown-linux-gnu/miri)
echo "Installing latest nightly with Miri: $MIRI_NIGHTLY"
rustup default "$MIRI_NIGHTLY"

rustup component add miri
cargo miri setup

cargo miri test --verbose
# This should probably be cargo clean right here
cargo miri test --verbose --all-features

Miri is run twice in a row with no cargo clean in between.

[ErisianArchitect]

I added cargo clean to the miri script. I don't know if that will fix the problem, but it's worth a try.