stdarch icon indicating copy to clipboard operation
stdarch copied to clipboard

Stabilize AArch64 SHA3 intrinsics

Open tarcieri opened this issue 4 months ago • 8 comments

See also: rust-lang/rust#117225

tarcieri avatar Mar 13 '24 16:03 tarcieri

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Amanieu (or someone else) some time within the next two weeks.

rustbot avatar Mar 13 '24 16:03 rustbot

These intrinsics are tested against clang and are known to produce the same results.

@rfcbot fcp merge

Amanieu avatar Mar 13 '24 17:03 Amanieu

@rfcbot fcp merge

Amanieu avatar Mar 13 '24 17:03 Amanieu

Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

  • [x] @Amanieu
  • [x] @BurntSushi
  • [x] @dtolnay
  • [ ] @joshtriplett
  • [ ] @m-ou-se

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

rfcbot avatar Mar 13 '24 18:03 rfcbot

@rfcbot reviewed

dtolnay avatar Mar 14 '24 01:03 dtolnay

The checkboxes are greyed out for me (I can't tick my box), but I approve.

BurntSushi avatar Mar 30 '24 11:03 BurntSushi

I've ticked your box.

Amanieu avatar Mar 30 '24 12:03 Amanieu

:bell: This is now entering its final comment period, as per the review above. :bell:

psst @Amanieu, I wasn't able to add the final-comment-period label, please do so.

rfcbot avatar Mar 30 '24 12:03 rfcbot

Merging master caused some build failures which seem to be unrelated:

https://github.com/rust-lang/stdarch/actions/runs/8491609619/job/23263755908?pr=1552#step:14:8910

   Compiling intrinsic-test v0.1.0 (/checkout/crates/intrinsic-test)
error: field `reg` is never read
  --> crates/intrinsic-test/src/json_parser.rs:21:9
   |
19 |     Register {
   |     -------- field in this variant
20 |         #[serde(rename = "register")]
21 |         reg: String,
   |         ^^^
   |
   = note: `-D dead-code` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(dead_code)]`

error: could not compile `intrinsic-test` (bin "intrinsic-test") due to 1 previous error
Error: Process completed with exit code 101.

tarcieri avatar Mar 30 '24 19:03 tarcieri

This seems to be fallout from https://github.com/rust-lang/rust/pull/119552 which was recently merged. Just add #[allow(dead_code)] on that field to ignore the lint.

Amanieu avatar Mar 31 '24 22:03 Amanieu

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

psst @Amanieu, I wasn't able to add the finished-final-comment-period label, please do so.

rfcbot avatar Apr 09 '24 12:04 rfcbot

:umbrella: The latest upstream changes (presumably 24068c78d625f8ddc8ffdabdb3aeb2dc644fac1e) made this pull request unmergeable. Please resolve the merge conflicts.

bors avatar Apr 10 '24 11:04 bors

Can you squash and rebase?

Amanieu avatar Apr 11 '24 23:04 Amanieu

@Amanieu done

tarcieri avatar Apr 11 '24 23:04 tarcieri