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

Explain additional rules for unsafe blocks in const and const fn #221

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jun 5, 2020

@@ -0,0 +1,171 @@
# Const safety

The miri engine, which is used to execute code at compile time, can fail in
Copy link
Member

Choose a reason for hiding this comment

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

miri or Miri? I have often been writing the latter, but not consistently...^^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've fairly consistently used the lower case form. 🤷 I have no opinion on it beyond "my fingers like not using more modifier keys than they need to".

Copy link
Member

@RalfJung RalfJung Jun 20, 2020

Choose a reason for hiding this comment

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

More importantly though, I feel the first sentence is not a great introduction here. I wrote it in the context of the const-eval repo, but this is different.

I'd start with something like "As part of the compilation process, the Rust compiler has to evaluate some of the code of a program. This is called CTFE (compile-time function evaluation). CTFE is used for: array lengths, const/static initializers, ...". Then say that CTFE can go wrong and just like at run-time, safe Rust protects programmers from some of the ways CTFE can fail. "Programmers only writing safe code do not have to care about any of this, but if you want to write unsafe code that is run by CTFE, then there are some extra concerns you need to be aware of."

Then the following text makes much more sense IMO.^^

@RalfJung
Copy link
Member

this is taken in large parts from https://github.com/rust-lang/const-eval/blob/master/const_safety.md

To avoid duplicating material, we should probably make that a link to the Nomicon after landing this.

src/const-safety.md Outdated Show resolved Hide resolved
src/const-safety.md Outdated Show resolved Hide resolved
src/const-safety.md Outdated Show resolved Hide resolved
oli-obk and others added 2 commits June 20, 2020 15:55
Co-authored-by: Ralf Jung <post@ralfj.de>
Co-authored-by: Ralf Jung <post@ralfj.de>
src/const-safety.md Outdated Show resolved Hide resolved
Co-authored-by: Ralf Jung <post@ralfj.de>
src/const-safety.md Outdated Show resolved Hide resolved
* fails to run at compile time when it works perfectly fine at runtime
* produces a different result at compile time than at runtime

is undefined behavior.
Copy link
Member

Choose a reason for hiding this comment

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

That would be a new kind of UB to be added to the reference, and to be checked by Miri. I think the latter will be really hard to do so I am not sure if it's worth it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, this is from before the guaranteed_* PR where I ended up agreeing that making it UB likely isn't worth it.

```

If the function were invoked as `ptr_eq(&42, &42)` the result depends on the potential
deduplication of the memory of the `42`s.
Copy link
Member

@RalfJung RalfJung Jun 20, 2020

Choose a reason for hiding this comment

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

Or rather, the result is that we get an error of the first kind: an unsupported operation. That is what the const-safety definition says must not happen.
The reason it is unsupported is that it cannot, in general, be supported -- but the text is skipping a step here.

@@ -7,6 +7,8 @@ The only things that are different in Unsafe Rust are that you can:
* Implement `unsafe` traits
* Mutate statics
* Access fields of `union`s
* Cast raw pointers to integers within const contexts
* Compare raw pointers within const contexts
Copy link
Member

Choose a reason for hiding this comment

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

With the guaranteed_* intrinsics, we could actually disallow the latter entirely, even in unsafe code, right?

@@ -18,6 +20,7 @@ language cares about is preventing the following things:

* Dereferencing (using the `*` operator on) dangling or unaligned pointers (see below)
* Breaking the [pointer aliasing rules][]
* Violating the [const safety][] rules
Copy link
Member

Choose a reason for hiding this comment

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

Uh-oh... this list is synced with this one and changing it is a big deal.^^

Copy link
Member

Choose a reason for hiding this comment

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

To be clear, what I am saying is that I think this PR should not change this list.

@RalfJung
Copy link
Member

So after the lang team design meeting that just happened, another alternative that emerged is to say that doing unconst things during CTFE is "simply" UB. And then we just have to describe what CTFE-UB is like, which we need to do anyway. I think that is actually my favorite plan so far. :D

Even if we adopt this (thus making most of the text here obsolete), some of the editing here could be incorporated into the const-eval version of this document.

@JohnTitor
Copy link
Member

Triage: It has been about a year since the last comment, I presume this has been waiting for @oli-obk to address the review comments, right?

@RalfJung
Copy link
Member

Also rust-lang/rfcs#3016 landed in the mean time (though I am not sure if that will affect this PR).

oli-obk and others added 2 commits July 20, 2021 12:59
Co-authored-by: Ralf Jung <post@ralfj.de>
Co-authored-by: Ralf Jung <post@ralfj.de>
a pointer (`Scalar::Ptr`) *or* a bunch of bits representing an integer
(`Scalar::Bits`), or uninitialized. Every value of a variable of primitive type is stored as a
`Scalar`. In the code above, casting the pointer `&S` to `*const i32` and then
to `usize` does not actually change the value -- we end up with a local variable
Copy link
Member

Choose a reason for hiding this comment

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

We don't allow ptr-to-int casts in const any more... should we adjust this story?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going through the entire doc, too many things changed. I'm keeping everything in mind that was commented here, but the entire doc needs an overhaul

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants