Partial progress on #132735: Replace extern "rust-intrinsic" with #[rustc_intrinsic] across the codebase #135333

vayunbiyani · 2025-01-10T13:52:14Z

Part of #132735: Replace extern "rust-intrinsic" with #[rustc_intrinsic] macro

Updated all instances of extern "rust-intrinsic" to use the #[rustc_intrinsic] macro.
Skipped .md files and test files to avoid unnecessary changes.

rustbot · 2025-01-10T13:52:24Z

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

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

rustbot · 2025-01-10T13:52:26Z

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred to the platform-builtins intrinsics. Make sure the
LLVM backend as well as portable-simd gets adapted for the changes.

cc @antoyo, @GuillaumeGomez, @bjorn3, @calebzulawski, @programmerjake

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @rust-lang/wg-const-eval

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

library/core/src/intrinsics/simd.rs

RalfJung · 2025-01-10T14:00:01Z

Thanks for the PR! However this definitely does not "fix" that issue, it's just next step. Also, have you coordinated with @BLANKatGITHUB who is assigned to that issue?

vayunbiyani · 2025-01-10T14:08:32Z

@RalfJung Thank you for the feedback!

I apologize for the confusion regarding the wording. You're right that this change is only part of the solution and doesn’t fully fix the issue.

I also overlooked that the issue is already assigned to @BLANKatGITHUB — my apologies for that. I’m happy to collaborate with them to move this forward. Given they mentioned being busy with exams, I’d also be happy to take ownership of the issue if that helps reduce their workload.

Please let me know how you'd like me to proceed. I’m keen to contribute further and ensure the issue is fully addressed.

RalfJung · 2025-01-10T14:09:04Z

Also please pick a proper PR title that summarizes the changes.

vayunbiyani · 2025-01-10T14:16:02Z

Apologies for the initial title — I’m new to contributing to a project as large as Rust. I've updated the title now!

Please let me know if you have any other feedback!

Veykril · 2025-01-10T14:01:31Z

src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/missing_unsafe.rs

    fn no_missing_unsafe_diagnostic_with_legacy_safe_intrinsic() {
        check_diagnostics(
            r#"
-extern "rust-intrinsic" {
-    #[rustc_safe_intrinsic]
-    pub fn bitreverse(x: u32) -> u32; // Safe intrinsic
-    pub fn floorf32(x: f32) -> f32; // Unsafe intrinsic
-}
+
+#[rustc_intrinsic]
+#[rustc_safe_intrinsic]
+pub fn bitreverse(x: u32) -> u32; // Safe intrinsic
+#[rustc_intrinsic]
+pub unsafe fn floorf32(x: f32) -> f32; // Unsafe intrinsic



This test specifically tests the old syntax so it needs to stay as is (we need to keep supporting it for temporary back-compat)

compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs

src/tools/rust-analyzer/crates/hir-ty/src/consteval/tests/intrinsics.rs

src/tools/rust-analyzer/crates/hir-ty/src/consteval/tests.rs

RalfJung

The miri changes LGTM.

RalfJung · 2025-01-16T17:01:56Z

src/tools/miri/tests/pass/intrinsics/portable-simd.rs

-}
+#[rustc_intrinsic]
+#[rustc_nounwind]
+pub fn simd_shuffle_generic<T, U, const IDX: &'static [u32]>(x: T, y: T) -> U;


It complains that the arguments are unused... that's unfortunate, ideally in an intrinsic without body we'd suppress that warning. Could you file an issue for that?

But for now, just add _ in front of every variable name, that is probably the simplest approach.

RalfJung · 2025-01-16T20:09:20Z

src/tools/miri/tests/pass/intrinsics/portable-simd.rs

@@ -16,7 +16,7 @@ use std::simd::prelude::*;

 #[rustc_intrinsic]
 #[rustc_nounwind]
-pub fn simd_shuffle_generic<T, U, const IDX: &'static [u32]>(x: T, y: T) -> U;
+pub fn simd_shuffle_generic<T, U, const IDX: &'static [u32]>(_x: T, _y: T) -> U;


This will be needed for all the Miri tests you changed.

RalfJung · 2025-01-16T20:50:22Z

You can locally test this with ./x test miri -- intrinsic. That should be better than always waiting for CI. :)

vayunbiyani · 2025-01-17T13:31:10Z

You can locally test this with ./x test miri -- intrinsic. That should be better than always waiting for CI. :)

Ah! Amazing, thanks! Appreciate you guiding me through it with such patience :)

RalfJung · 2025-01-18T09:24:30Z

There are still some unresolved comments further up. It may be better, in fact, to just not touch rust-analyzer at all here? They probably want to remove support for this syntax at their own pace.

vayunbiyani · 2025-01-19T06:38:39Z

@RalfJung This looks good to merge. Could you kindly review it?

RalfJung · 2025-01-19T17:06:55Z

The change looks good, thanks! Please squash the history into a single commit to get rid of the back-and-forth.

RalfJung · 2025-01-19T17:07:05Z

@rustbot author
Please post @rustbot ready when the PR is ready for the next round of review.

…acy extern "rust-intrinsic" blocks

vayunbiyani · 2025-01-20T17:11:37Z

@rustbot ready

RalfJung · 2025-01-20T18:47:25Z

@bors r+ rollup

bors · 2025-01-20T18:47:27Z

📌 Commit c79fc90 has been approved by RalfJung

It is now in the queue for this repository.

RalfJung · 2025-01-20T18:48:15Z

You wrote "fix #132735" in the PR description -- that's not accurate; the PR makes progress on the issue but more work remains. "fix" makes github close the issue, so please only use that when the PR really fully resolves an issue.

vayunbiyani · 2025-01-20T19:09:23Z

You wrote "fix #132735" in the PR description -- that's not accurate; the PR makes progress on the issue but more work remains. "fix" makes github close the issue, so please only use that when the PR really fully resolves an issue.

Apologies, I'll keep that in mind going forward!

…lfJung Partial progress on rust-lang#132735: Replace extern "rust-intrinsic" with #[rustc_intrinsic] across the codebase Part of rust-lang#132735: Replace `extern "rust-intrinsic"` with `#[rustc_intrinsic]` macro - Updated all instances of `extern "rust-intrinsic"` to use the `#[rustc_intrinsic]` macro. - Skipped `.md` files and test files to avoid unnecessary changes.

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#133695 (Reexport likely/unlikely in std::hint) - rust-lang#135330 (Respect --sysroot for rustc -vV and -Cpasses=list) - rust-lang#135333 (Partial progress on rust-lang#132735: Replace extern "rust-intrinsic" with #[rustc_intrinsic] across the codebase) - rust-lang#135741 (Recognise new IPv6 documentation range from IETF RFC 9637) - rust-lang#135770 (Update contributing docs for submodule/subtree changes) - rust-lang#135775 (Subtree update of `rust-analyzer`) - rust-lang#135776 (Subtree sync for rustc_codegen_cranelift) r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned compiler-errors Jan 10, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jan 10, 2025

cyrgani reviewed Jan 10, 2025

View reviewed changes

library/core/src/intrinsics/simd.rs Outdated Show resolved Hide resolved

vayunbiyani changed the title ~~changes in all rs files~~ Partial progress on #132735: Replace extern "rust-intrinsic" with #[rustc_intrinsic] across the codebase Jan 10, 2025

Veykril suggested changes Jan 10, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

bjorn3 reviewed Jan 10, 2025

View reviewed changes

compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs Outdated Show resolved Hide resolved

oli-obk reviewed Jan 10, 2025

View reviewed changes

src/tools/rust-analyzer/crates/hir-ty/src/consteval/tests/intrinsics.rs Outdated Show resolved Hide resolved

oli-obk reviewed Jan 10, 2025

View reviewed changes

src/tools/rust-analyzer/crates/hir-ty/src/consteval/tests.rs Outdated Show resolved Hide resolved

RalfJung reviewed Jan 10, 2025

View reviewed changes

vayunbiyani marked this pull request as draft January 10, 2025 14:56

This comment has been minimized.

Sign in to view

RalfJung reviewed Jan 16, 2025

View reviewed changes

vayunbiyani mentioned this pull request Jan 16, 2025

Unused Argument Warnings in Intrinsics Without Body #135598

Open

RalfJung reviewed Jan 16, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

vayunbiyani marked this pull request as ready for review January 18, 2025 13:21

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 19, 2025

Updated several files to use rust intrinsic macros instead of the leg…

c79fc90

…acy extern "rust-intrinsic" blocks

vayunbiyani force-pushed the test-environment branch from ecb99f9 to c79fc90 Compare January 20, 2025 14:18

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 20, 2025

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 20, 2025

matthiaskrgr mentioned this pull request Jan 20, 2025

Rollup of 8 pull requests #135788

Closed

matthiaskrgr mentioned this pull request Jan 20, 2025

Rollup of 7 pull requests #135789

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Partial progress on #132735: Replace extern "rust-intrinsic" with #[rustc_intrinsic] across the codebase #135333

Partial progress on #132735: Replace extern "rust-intrinsic" with #[rustc_intrinsic] across the codebase #135333

vayunbiyani commented Jan 10, 2025 •

edited by RalfJung

Loading

rustbot commented Jan 10, 2025

rustbot commented Jan 10, 2025

RalfJung commented Jan 10, 2025 via email

vayunbiyani commented Jan 10, 2025

RalfJung commented Jan 10, 2025 via email

vayunbiyani commented Jan 10, 2025

Veykril Jan 10, 2025 •

edited

Loading

This comment has been minimized.

RalfJung left a comment

This comment has been minimized.

This comment has been minimized.

RalfJung Jan 16, 2025

vayunbiyani Jan 16, 2025

RalfJung Jan 16, 2025

This comment has been minimized.

RalfJung commented Jan 16, 2025

This comment has been minimized.

This comment has been minimized.

vayunbiyani commented Jan 17, 2025

RalfJung commented Jan 18, 2025

vayunbiyani commented Jan 19, 2025

RalfJung commented Jan 19, 2025

RalfJung commented Jan 19, 2025

vayunbiyani commented Jan 20, 2025

RalfJung commented Jan 20, 2025

bors commented Jan 20, 2025

RalfJung commented Jan 20, 2025

vayunbiyani commented Jan 20, 2025

Partial progress on #132735: Replace extern "rust-intrinsic" with #[rustc_intrinsic] across the codebase #135333

Are you sure you want to change the base?

Partial progress on #132735: Replace extern "rust-intrinsic" with #[rustc_intrinsic] across the codebase #135333

Conversation

vayunbiyani commented Jan 10, 2025 • edited by RalfJung Loading

rustbot commented Jan 10, 2025

rustbot commented Jan 10, 2025

RalfJung commented Jan 10, 2025 via email

vayunbiyani commented Jan 10, 2025

RalfJung commented Jan 10, 2025 via email

vayunbiyani commented Jan 10, 2025

Veykril Jan 10, 2025 • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

RalfJung left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

RalfJung Jan 16, 2025

Choose a reason for hiding this comment

vayunbiyani Jan 16, 2025

Choose a reason for hiding this comment

RalfJung Jan 16, 2025

Choose a reason for hiding this comment

This comment has been minimized.

RalfJung commented Jan 16, 2025

This comment has been minimized.

This comment has been minimized.

vayunbiyani commented Jan 17, 2025

RalfJung commented Jan 18, 2025

vayunbiyani commented Jan 19, 2025

RalfJung commented Jan 19, 2025

RalfJung commented Jan 19, 2025

vayunbiyani commented Jan 20, 2025

RalfJung commented Jan 20, 2025

bors commented Jan 20, 2025

RalfJung commented Jan 20, 2025

vayunbiyani commented Jan 20, 2025

vayunbiyani commented Jan 10, 2025 •

edited by RalfJung

Loading

Veykril Jan 10, 2025 •

edited

Loading