Rust: Create newtype structs for typedefs

[divergentdave]

This PR fixes #1620 by replacing type aliases for arrays with newtypes. As the types for Montgomery/non-Montgomery or tight/loose field elements are now distinct, this will prevent misuse by consumers. Since this is a breaking change, I bumped the crate version to 0.2.0.

I couldn't figure out how to make the suggested changes to stringification of IR.List_nth expressions, so I solved that part of the problem in Rust instead. Bare arrays, without typedefs, are still used in function arguments. Thus, any IR.List_nth operation would either need to be lowered to ({v}[{n}]) or ({v}.0[{n}]), depending on the type of the operation's argument. I couldn't figure out how to access type information starting from these IR nodes, so I instead implemented Index and IndexMut on the new Rust structs, in order to forward indexing operations to the wrapped arrays.

FYI, I test-ran the Rust CI scripts locally, and I noted that in the curve25519-dalek and Ed448-Goldilocks tests, Cargo is ignoring the patched dependencies because of version number mismatches. (path = "../fiat-rust" has version 0.2.0, while the two tested crates depend on 0.1, so Cargo still uses the original crate from crates.io)

[andres-erbsen]

Thank you for the contribution. I will do my best to help this get merged, working with my limited knowledge of rust best practices.

The version and implementation changes look good. Indeed, the stringification IR is unfortunately untyped. Defining indexing on the wrapper struct seems fine even without the need for a workaround here, so I am happy to go with that. (If you'd prefer not to, I don't think the unknown-length case is actually used anywhere, so a quick alternative may be to make the type declaration for that error and change the pretty-printing of indexing in a way that only works for wrapped fixed-length arrays.)

Thanks for the heads-up about tests. I am not sure what the right thing to do for integration-testing breaking changes in Cargo packages is, but I think it would make sense to attempt to update the relevant crates and not leave fiat-rust untested. I guess the ideal scenario here would be to open PRs against the crates used in tests, perhaps following "Prepublishing a breaking change" with your feature branch. Then once the tests pass (on their CI or ours), this change could be merged with confidence, and downstream PRs could be reverted to refer to the usual fiat-crypto repository before merging. In the lucky case that downstreams don't need source-code changes, all this would be a little bit redundant, but probably still fine.

Do you think you could take a stab at this and expect success without too much effort? I am overall agnostic about whether fiat-crypto should be testing against reverse dependencies, so adding some appropriate tests in this repo could be an alternative.

[divergentdave]

I've got the Ed448-Goldilocks changes up in a PR, and made a first draft of changes for curve25519-dalek, but the latter results in significant benchmark regressions. I'll have to look at profiling and assembly listings closely, and maybe change how I'm initializing and copying things.

[divergentdave]

Good news: adding a #[inline] on index() and index_mut() took care of my benchmark regressions. I've updated the test scripts to run against PR branches of downstream dependencies for now, and they're passing locally.

[JasonGross]

These are temporary and should go away before this is merged, right?

[divergentdave]

That depends on what we want to do with these tests against reverse dependencies. If we make no change to these scripts, then they would only test the head of Ed448-Goldilocks and curve25519-dalek against fiat-crypto 0.1.20 from crates.io (see discussion above). With the changes I have here currently, they would test pinned commits of Ed448-Goldilocks and curve25519-dalek against the head of this repository. Once each downstream crate upgrades, its test script could be rolled back, in order to test the head of that repository against the head of this repository. The third option would be to rip out tests against the other repositories and add an independent set of tests here. (see @andres-erbsen's comment)

[divergentdave]

I removed the pinned commits, let me know what you think.

[andres-erbsen]

@JasonGross can you bump https://crates.io/crates/fiat-crypto/? (How does this work?)

[JasonGross]

There's a CI job that bumps it whenever we tag a release

[JasonGross]

I'll tag one now

[JasonGross]

I've tagged https://github.com/mit-plv/fiat-crypto/releases/tag/v0.0.21, https://github.com/mit-plv/fiat-crypto/actions/runs/6032879535/job/16368724510 is bumping the rust crate