feat: make rust const#2080
Conversation
| let x23: u128 = (((arg1[0]) as u128) * ((arg2[2]) as u128)); | ||
| let x24: u128 = (((arg1[0]) as u128) * ((arg2[1]) as u128)); | ||
| let x25: u128 = (((arg1[0]) as u128) * ((arg2[0]) as u128)); | ||
| pub const fn fiat_25519_carry_mul(mut out1: &mut fiat_25519_tight_field_element, arg1: &fiat_25519_loose_field_element, arg2: &fiat_25519_loose_field_element) { |
There was a problem hiding this comment.
Why do we need mut out1 now? And will this break any downstream crates?
There was a problem hiding this comment.
The short answer is "no, everything is fine". mut in only part of the internal signature of out1 not the signature of fiat_25519_carry_mul.
The reason why out1 need to be mut is a quirk of how rust mutable borrowing works. There are multiple indexing operations into out1, which means multiple IndexConsts are constructed. IndexConst take ownership of whatever is passed in, which is fine when dealing with & which are Copy, but &mut is not Copy. To get around this we borrow out1 and pass that in instead. The issue is if we used &out1 to borrow, then we couldn't get &mut out because once you're non-mut you're stuck that way. So instead we use &mut out1, but for that to work the local variable out1 must be mutable.
As state above though, this doesn't impact callers because it's only the local variable out1 which is being changed to be mutable. It's the same as putting let mut out1 = out1; as the first line and shadowing the variable for the whole function.
| @@ -87,6 +101,23 @@ Module Rust. | |||
| ([""; | |||
| "#![allow(unused_parens)]"; | |||
| "#![allow(non_camel_case_types)]"; | |||
| ""; | |||
| "struct IndexConst<T: ?Sized>(T);"; | |||
There was a problem hiding this comment.
It might be worth adding a Rust comment here about why this type exists (basically just the contents of the PR message)
Co-authored-by: Jason Gross <jasongross9@gmail.com>
2 participants
&mutisconst-stable as of rust1.83. As this is a pretty big jump in MSRV, it should probably be a minor version bump.Unfortunately using
IndexandIndexMutinconstfunctions isn'tconst, except on built-in arrays. However you can get around this with an internal helper type (IndexConstin this PR) doing the array indexing. This does make theIndexandIndexMutimpls internally redundant, but they're harmless and removing them would break downstream crates, which would be a major version bump.At some point in the distant future when
const traits are stabilized,IndexConstcan go away and the original code will just work with essentially no change to public API. But that's probably a few years away.