Skip to content
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

Add statements on intrinsics. #148

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

Add statements on intrinsics. #148

Changes from 1 commit
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
13 changes: 13 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,13 @@

# 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.


For many, there is already stabilized API available.

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".


With `transmute` there is one major exception. However, `transmute` is handled in other parts of the guidelines.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what the exception being referred to here is,

  • transmute is exceptional because it isn't Experimental
    • This is different from most other intrinsics, but copy, copy_nonoverlapping, and write_bytes are also not marked as Experimental
  • transmute is exceptional because it doesn't mention a stabilized API in its documentation
    • There is std::mem::transmute, however I guess this is just a re-export of the instrinsic not an API
    • copy, copy_nonoverlapping, and write_bytes don't also reference a stabilized API, and have a std::ptr re-export
  • transmute is exceptional because it is an experimental compiler intrinsic that should not be avoided in a safety-critical project
    • It isn't experimental, so the above guidance doesn't really apply
    • The Rustonomicon also lists uses for copy and copy_nonoverlapping's re-exports when assigning to a memory location without invoking a drop

Copy link
Collaborator

Choose a reason for hiding this comment

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

ping @vjonaswolf for his feedback on @vapdrs's comment