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.

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

Conversation

vjonaswolf
Copy link
Contributor

No description provided.

@vjonaswolf
Copy link
Contributor Author

@vapdrs would you like to have a look at my minimal first version?

@PLeVasseur
Copy link
Collaborator

Hey @vjonaswolf -- thanks for kicking things off :)

Could you add in a small description to the PR summary, so we can have a bit of traceability later?

Copy link
Contributor

@vapdrs vapdrs left a comment

Choose a reason for hiding this comment

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

Thank you for putting this together. Overall I'd say this covers the necessary basics for this topic. Some "nice to have" additions would a code example or link to one for transmute, and a statement on the considerations that should be taken for safety when using intrinics. If you would like me to draft some wording for those, let me know.


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

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

Copy link
Contributor

@vapdrs vapdrs left a comment

Choose a reason for hiding this comment

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

Looks good to me

@vjonaswolf
Copy link
Contributor Author

closes #174

Copy link
Collaborator

@PLeVasseur PLeVasseur left a comment

Choose a reason for hiding this comment

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

Hey @vjonaswolf -- thanks for taking this on! I've left a few notes poking around how much scope we see being in this chapter.


Most of them are marked as experimental, unstable and useable 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?

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

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.


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.

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?

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

Choose a reason for hiding this comment

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

Yes, please cross link.


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

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?

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.

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.

@PLeVasseur PLeVasseur added documentation Improvements or additions to documentation coding guidelines Related to work in the Coding Guidelines Subcommittee labels Feb 13, 2025

# Intrinsics

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

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.

Copy link
Contributor

@iglesias iglesias left a comment

Choose a reason for hiding this comment

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

Hello. I wrote suggestions to fix a few typos in case they may help.


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

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


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

Most of them are marked as experimental, unstable and useable in nightly versions of the compiler.
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.


Most of them are marked as experimental, unstable and useable 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
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
coding guidelines Related to work in the Coding Guidelines Subcommittee documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants