use const generics for the PackedCompare
control byte
#39
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi Jake 👋.
I was looking at this crate in the stdarch crater results, and I'd thought I'd try to fix it since it's yours :) I'm not sure if it's still super active, or if the proposed solution would be acceptable to you (as it uses const generics and thus is technically a breaking change by bumping the MSRV). There may be other acceptable solutions of course, but this seemed clean enough to ask for your opinion.
Some generic explanations follow:
Your crate came up in the results of a crater run updating a part of
libcore
:stdarch
, and this PR should fix that.For context, we're trying to update
libcore
'sstdarch
to improve handling of immediate mode arguments (tracking issue: rust-lang/stdarch#248): the current solution is suboptimal. It uses macros to ensure the immediate arguments are indeed constants, and that causes problems (the size oflibcore
and the time it takes to compile).Now that const generics are available, we're trying to move from the previous way of doing things to using const generics, so that non-constant immediates cause a compile error instead:
#[rustc_legacy_const_generics]
mechanism to keep the unexpectedly stable intrinsics using the old method working (butmin_const_generics
are not exactly equivalent: associated consts require the fullconst_generics
). So this would be a breaking change for the few crates where this const generics difference is visible, likejetscii
.stdarch
were updated (tracking issue for this effort: Remove all uses of#[rustc_args_required_const]
rust-lang/stdarch#1022)stdarch
is currently being tested in crater, via the Bump stdarch submodule rust-lang/rust#83278 PR.(There is also more discussion around this topic in rust-lang/rust#83167 if that interests you, but it is more about the best way to keep existing code working, whether a switch to const generics should be done over an edition, and so on.)
Overall,
jetscii
would stop compiling if that PR above were to be merged: thePackedCompare::cmpestrm{i}
methods are generic and use the_mm_cmpestrm{i}
intrinsics. However, they pass the control byte immediate via a generic constant and that doesn't work withmin_const_generics
and#[rustc_legacy_const_generics]
.So, this PR adds a const generic parameter to the
PackedCompare
struct, and removes theCONTROL_BYTE
associated constant fromPackedCompareControl
. It's less self-contained than before, that's for sure...I've ran the tests in this crate using the rustc CI artifacts from that PR: thanks to
rustup-toolchain-install-master
, with the9cae4dca0b4ddbfe82616f4d5f1090fc817e2ffa
rustc CI revision.