Skip to content

Update some comment/docs related to "extern intrinsic" removal #139490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions library/core/src/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,11 @@
//!
//! # Const intrinsics
//!
//! Note: any changes to the constness of intrinsics should be discussed with the language team.
//! This includes changes in the stability of the constness.
//!
//! //FIXME(#132735) "old" style intrinsics support has been removed
//! In order to make an intrinsic usable at compile-time, it needs to be declared in the "new"
//! style, i.e. as a `#[rustc_intrinsic]` function, not inside an `extern` block. Then copy the
//! implementation from <https://github.com/rust-lang/miri/blob/master/src/intrinsics> to
//! In order to make an intrinsic unstable usable at compile-time, copy the implementation from
//! <https://github.com/rust-lang/miri/blob/master/src/intrinsics> to
//! <https://github.com/rust-lang/rust/blob/master/compiler/rustc_const_eval/src/interpret/intrinsics.rs>
//! and make the intrinsic declaration a `const fn`.
//! and make the intrinsic declaration below a `const fn`. This should be done in coordination with
//! wg-const-eval.
//!
//! If an intrinsic is supposed to be used from a `const fn` with a `rustc_const_stable` attribute,
//! `#[rustc_intrinsic_const_stable_indirect]` needs to be added to the intrinsic. Such a change requires
Expand Down
3 changes: 2 additions & 1 deletion src/doc/unstable-book/src/language-features/intrinsics.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ Various intrinsics have native MIR operations that they correspond to. Instead o
backends to implement both the intrinsic and the MIR operation, the `lower_intrinsics` pass
will convert the calls to the MIR operation. Backends do not need to know about these intrinsics
at all. These intrinsics only make sense without a body, and can be declared as a `#[rustc_intrinsic]`.
The body is never used, as calls to the intrinsic do not exist anymore after MIR analyses.
The body is never used as the lowering pass implements support for all backends, so we never have to
use the fallback logic.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rust-lang/rust-analyzer I guess this PR will fail to land since RA has a copy of this file for the hover_feature test. How can I adjust the test to not require such a hard-to-maintain full copy?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a feature that won't really ever change maybe that we can just swap to?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah well, replacing the hover test with

#[test]
fn hover_feature() {
    let (analysis, position) = fixture::position(r#"#![feature(intrinsics$0)]"#);
    analysis
        .hover(
            &HoverConfig { links_in_hover: true, ..HOVER_BASE_CONFIG },
            FileRange { file_id: position.file_id, range: TextRange::empty(position.offset) },
        )
        .unwrap()
        .unwrap();
}

should suffice

Copy link
Member Author

@RalfJung RalfJung Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As someone who has ran into tests relying on "surely this unstable feature will never change" about a dozen times over the years, and each time had to do a frustrating search for what this test actually cared about and what was entirely incidental reliance on a feature -- I'd prefer if we didn't go down that route. Why does the test need to compare the full string for equality? There's no reason for that that I can think of.

If absolutely necessary we could probably have a dedicated feature that exists solely for the purpose of testing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah well, replacing the hover test with

Ah, that's great, thanks a lot. :)


## Intrinsics without fallback logic

Expand Down
87 changes: 8 additions & 79 deletions src/tools/rust-analyzer/crates/ide/src/hover/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6882,85 +6882,14 @@ pub fn foo() {}

#[test]
fn hover_feature() {
check(
r#"#![feature(intrinsics$0)]"#,
expect![[r#"
*intrinsics*
```
intrinsics
```
___

# `intrinsics`

The tracking issue for this feature is: None.

Intrinsics are rarely intended to be stable directly, but are usually
exported in some sort of stable manner. Prefer using the stable interfaces to
the intrinsic directly when you can.

------------------------


## Intrinsics with fallback logic

Many intrinsics can be written in pure rust, albeit inefficiently or without supporting
some features that only exist on some backends. Backends can simply not implement those
intrinsics without causing any code miscompilations or failures to compile.
All intrinsic fallback bodies are automatically made cross-crate inlineable (like `#[inline]`)
by the codegen backend, but not the MIR inliner.

```rust
#![feature(intrinsics)]
#![allow(internal_features)]

#[rustc_intrinsic]
const unsafe fn const_deallocate(_ptr: *mut u8, _size: usize, _align: usize) {}
```

Since these are just regular functions, it is perfectly ok to create the intrinsic twice:

```rust
#![feature(intrinsics)]
#![allow(internal_features)]

#[rustc_intrinsic]
const unsafe fn const_deallocate(_ptr: *mut u8, _size: usize, _align: usize) {}

mod foo {
#[rustc_intrinsic]
const unsafe fn const_deallocate(_ptr: *mut u8, _size: usize, _align: usize) {
panic!("noisy const dealloc")
}
}

```

The behaviour on backends that override the intrinsic is exactly the same. On other
backends, the intrinsic behaviour depends on which implementation is called, just like
with any regular function.

## Intrinsics lowered to MIR instructions

Various intrinsics have native MIR operations that they correspond to. Instead of requiring
backends to implement both the intrinsic and the MIR operation, the `lower_intrinsics` pass
will convert the calls to the MIR operation. Backends do not need to know about these intrinsics
at all. These intrinsics only make sense without a body, and can be as a `#[rustc_intrinsic]`.
The body is never used, as calls to the intrinsic do not exist anymore after MIR analyses.

## Intrinsics without fallback logic

These must be implemented by all backends.

### `#[rustc_intrinsic]` declarations

These are written like intrinsics with fallback bodies, but the body is irrelevant.
Use `loop {}` for the body or call the intrinsic recursively and add
`#[rustc_intrinsic_must_be_overridden]` to the function to ensure that backends don't
invoke the body.

"#]],
)
let (analysis, position) = fixture::position(r#"#![feature(intrinsics$0)]"#);
analysis
.hover(
&HoverConfig { links_in_hover: true, ..HOVER_BASE_CONFIG },
FileRange { file_id: position.file_id, range: TextRange::empty(position.offset) },
)
.unwrap()
.unwrap();
}

#[test]
Expand Down
Loading