Skip to content

Add statements on intrinsics. #148

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add statements on intrinsics. #148

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c33f1b1
Add statements.
vjonaswolf Jan 17, 2025
ab3e76c
Update subcommittee/coding-guidelines/initiatives/safe-use-of-unsafe-…
vjonaswolf Feb 10, 2025
bfb9b03
Improve based on review comments.
vjonaswolf Feb 12, 2025
748c5b5
Merge branch 'guideline-on-intrinsics' of https://github.com/vjonaswo…
vjonaswolf Feb 12, 2025
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
15 changes: 15 additions & 0 deletions ...safe-use-of-unsafe-guidelines/learn_unsafe_rust/src/core_unsafety/intrinsics.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@

# Intrinsics

Rust provides a set of compiler intrinsics as part of `core` and `std` respectively (https://doc.rust-lang.org/std/intrinsics/index.html).
Copy link

Choose a reason for hiding this comment

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

At a minimum I'd expect this chapter to go into some depth about what an intrinsic is, and how it is different from a regular function.


Most of them are marked as experimental, unstable and useable in nightly versions of the compiler.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would have expected some discussion of std::arch, the "SIMD and vendor intrinsics module".

I know that most usage is behind crates which use std::arch, but given the level we're working at in safety-critical software, we sometimes need to drop down to that level of specificity.

Do you think it's useful to include a std::arch discussion here and how unsafe factors in?

Copy link

Choose a reason for hiding this comment

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

I think simd should be a separate chapter.

Copy link

Choose a reason for hiding this comment

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

This chapter can link to it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Most of them are marked as experimental, unstable and useable in nightly versions of the compiler.
Most of them are marked as experimental, unstable and usable in nightly versions of the compiler.


Many intrinsics are accessible though a stabilized API located elsewhere in the Standard or Core Library. The documentation for each intrinsic typically references the stabilized API.
Copy link
Collaborator

Choose a reason for hiding this comment

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

It may be useful to break this out into its own section and highlight some examples. What do you think?

Copy link

Choose a reason for hiding this comment

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

Yes, there should be plenty of examples

Copy link

Choose a reason for hiding this comment

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

Really, I'd say there should be an ontology of intrinsics here: a categorization of the stdlib intrinsics into different types.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Many intrinsics are accessible though a stabilized API located elsewhere in the Standard or Core Library. The documentation for each intrinsic typically references the stabilized API.
Many intrinsics are accessible through a stabilized API located elsewhere in the Standard or Core Library. The documentation for each intrinsic typically references the stabilized API.


Thus, the clear recommendation is not using the experimental compiler intrinsics in a safety-critical project.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I can understand this general recommendation! Perhaps those exposed via other core or std APIs would be useful to discuss and note where undefined behavior can crop up?

Copy link

Choose a reason for hiding this comment

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

I don't think this is the right way to approach this. As mentioned before, these chapters must:

  • Talk about how the safety works for this feature
  • Talk about things to think about when using them
  • Talk about when one might need to reach for them
  • Talk about best practices when you are reaching for them (this is where you say "try using the stabilized APIs instead" for some of these APIs)

This is unsafe code: people are already in a weird situation. They may have a good reason for reaching for an intrinsic, perhaps it's new and unstabilized, perhaps it's one of the more niche ones that never got a stable API, perhaps it's one of the atomic ones which have higher level stable APIs but not the lower level direct atomic accesses.

It should not be our job to say "don't do this", it should be our job to say "if you need to do this, here's how you make sure you actually need to do this, and here's how to make sure you're doing it right".

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is unsafe code: people are already in a weird situation.

I think this is fair.

@vjonaswolf -- perhaps we should take the safety-critical lens off here for the moment and broaden to helping people in a weird situation.

As we talked about a bit with @adfernandes, some of the work he's doing on the safety-critical pamphlet can help with noting in resources we reference like Learn unsafe Rust the context and certain things / sections to avoid or suggest alternatives to like "hey try to use these stabilized APIs instead, if possible".


There are only few functions that are not marked as experimental: `copy`, `copy_nonoverlapping`, `write_bytes`, `transmute`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
There are only few functions that are not marked as experimental: `copy`, `copy_nonoverlapping`, `write_bytes`, `transmute`.
There are only a few functions that are not marked as experimental: `copy`, `copy_nonoverlapping`, `write_bytes`, `transmute`.

`transmute` is probably the most well-known and widely used of these APIs.
`transmute` is handled in other parts of the guidelines.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you refer to the chapter on Invalid Values, particularly this section?

If so, it might be nice to make the link over there explicit.

Copy link

Choose a reason for hiding this comment

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

Yes, please cross link.