-
Notifications
You must be signed in to change notification settings - Fork 17
add document on consts in patterns #48
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
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
266a7ad
add document on consts in patterns
RalfJung 81a6b8e
link to reading-mutable-static soundness concern
RalfJung e8414f1
add some links to code
RalfJung c67ebb5
document for which consts we do exhaustiveness checking
RalfJung bd37599
Explain how patterns are compiled
RalfJung 18f3020
compare with RFC 1445
RalfJung File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
# Constants in patterns | ||
|
||
Constants that pass the [structural equality check][struct-eq] can be used in patterns: | ||
```rust | ||
const FOO: i32 = 13; | ||
|
||
fn is_foo(x: i32) -> bool { | ||
match x { | ||
FOO => true, | ||
_ => false, | ||
} | ||
} | ||
``` | ||
However, that check has some loopholes, so e.g. `&T` can be used in a pattern no matter the `T`. | ||
|
||
Any reference type const-pattern is compiled by [calling `PartialEq::eq`][compile-partial-eq] to compare the subject of the `match` with the constant. Const-patterns with other types (enum, struct, tuple, array) are treated as if the constant was inlined as a pattern (and the usual `match` tree is constructed). | ||
[RFC 1445](https://github.com/rust-lang/rfcs/blob/master/text/1445-restrict-constants-in-patterns.md) lays the groundwork for changing this in the future to always using `PartialEq::eq`, but no decision either way has been made yet. | ||
|
||
[struct-eq]: https://github.com/rust-lang/rust/blob/2c28244cf0fc9868f55070e55b8f332d196eaf3f/src/librustc_mir_build/hair/pattern/const_to_pat.rs#L121 | ||
[compile-partial-eq]: https://github.com/rust-lang/rust/blob/2c28244cf0fc9868f55070e55b8f332d196eaf3f/src/librustc_mir_build/build/matches/test.rs#L355 | ||
|
||
## Soundness concerns | ||
|
||
Most of the time there are no extra soundness concerns due to const-patterns; except for surprising behavior nothing can go wrong when the structural equality check gives a wrong answer. | ||
However, there is one exception: some constants participate in exhaustiveness checking. | ||
If a pattern is incorrectly considered exhaustive, that leads to a critical soundness bug. | ||
|
||
Exhaustiveness checking is done for all constants that do not have reference type, as well as `&[u8]` and `&str`. | ||
This means we can write: | ||
```rust | ||
#![feature(exclusive_range_pattern)] | ||
#![feature(half_open_range_patterns)] | ||
const PAT: &[u8] = &[0]; | ||
|
||
pub fn test(x: &[u8]) -> bool { | ||
match x { | ||
PAT => true, | ||
&[] => false, | ||
&[1..] => false, | ||
&[_, _, ..] => false | ||
} | ||
} | ||
``` | ||
|
||
To ensure soundness of exhaustiveness checking, it is crucial that all data considered this check is fully immutable. | ||
In particular, for constants of reference type, it is important that they only point to immutable data. | ||
For this reason, the static const checks reject references to `static` items. | ||
This is a new soundness concern that otherwise does not come up during CTFE. | ||
Note that this is orthogonal to the concern of [*reading from* a mutable `static`](const.md#reading-statics) during const initialization, which is a problem for all `const` (even when they are not used in patterns) because it breaks the "inlining" property. | ||
A more precise check could be possible, but is non-trivial: even an immutable `static` could point to a mutable `static`; that would have to be excluded. | ||
(We could, in principle, also rely on it being UB to have a shared reference to mutable memory, but for now we prefer not to rely on the aliasing model like that---aliasing of global pointers is a tricky subject.) | ||
|
||
*Dynamic check.* | ||
To dynamically ensure constants only point to read-only data, we maintain a global invariant: | ||
pointers to a `static` (including memory that was created as part of interning a `static`) may never "leak" to a non-`static`. | ||
All memory outside of `static`s is marked as immutable. | ||
As a consequence, non-`static`s recursively only point to immutable memory. | ||
|
||
This check is uncomfortably close to the static checks, and fragile due to its reliance on a global invariant. | ||
Longer-term, it would be better to move to a more local check, by making validation check if consts are recursively read-only. | ||
This check (unlike our current validation) would have to descend through pointers to `static`. | ||
|
||
An alternative could be to wait for the [value tree proposal](https://github.com/rust-lang/compiler-team/issues/323) to be implemented. | ||
Then we could check during value tree construction that all memory is read-only (if we use the CTFE machine in `const`-mode, that will happen implicitly), and thus we know for sure we can rely on the value tree to be sound to use for exhaustiveness checking. | ||
How exactly this looks like depends on how we resolve [pattern matching allowing many things that do not fit a value tree](https://github.com/rust-lang/rust/issues/74446#issuecomment-663439899). | ||
However, if only the well-behaved part of the value tree is used for exhaustiveness checking, handling these extra patterns should not interact closely with the soundness concerns discussed here. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.