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

"_" patterns and validity invariants #261

Open
RalfJung opened this issue Dec 5, 2020 · 20 comments
Open

"_" patterns and validity invariants #261

RalfJung opened this issue Dec 5, 2020 · 20 comments
Labels
A-validity Topic: Related to validity invariants S-pending-documentation Status: The issue is "resolved," but this resolution needs documentation

Comments

@RalfJung
Copy link
Member

RalfJung commented Dec 5, 2020

The following code is not even unsafe, so it certainly cannot be UB:

fn foo(ptr: *const bool) {
    let _ = *ptr;
}

foo(&3u8 as *const u8 as *const bool);

This indicates that the pointer does not really get dereferenced, so the pointed-to data is never interpreted at type bool and thus must not be valid. I am not sure I like this; *ptr (as a value expression!) to me seems like it should conceptually perform the load even if the result is later discarded.

Something similar happens here:

fn bar(ptr: *const (i8, bool)) { unsafe {
    let (x, _) = *ptr;
} }

This compiles effectively to let x = (*ptr).0, so again whatever data is in the second field does not matter. (Note that *ptr here occurs only as a place expression, not as a value expression, so it makes sense that in (*ptr).0 we do not require the second field to be valid.)

I think this behavior of "_" can be explained by saying that it suppresses the place-to-value coercion, and it is smart enough to even do this in cases like (x, _) where the coercion is partially suppressed. But I am not sure I like this semantics, it seems rather counter-intuitive to me. It also makes patterns much harder to explain. And it leads to strange special cases such as rust-lang/rust#79735 (I cannot tell if this is some compiler author also being confused by "_", or if the intention is that ! is special in that even creating the place is already UB, or something else).

@RalfJung RalfJung changed the title "_" patterns are strange "_" patterns and validity invariants Dec 5, 2020
@RalfJung RalfJung added C-open-question Category: An open question that we should revisit A-validity Topic: Related to validity invariants labels Dec 5, 2020
@RalfJung
Copy link
Member Author

RalfJung commented Dec 5, 2020

@Nadrieril points out that

Right now you are allowed to write let (y, _) = x if we moved out x.1 already. The corresponding match is not allowed tho

@RalfJung
Copy link
Member Author

RalfJung commented Dec 5, 2020

The corresponding case for partial initialization can currently not be probed since rustc no longer permits writing to individual fields of a not-yet-initialized struct:

fn main() {
    let x: (i32, bool);
    x.0 = 5; // this already errors here...
    let (y, _) = x; // ... so we cannot test if this would be accepted
}

If this code was accepted, that would be further evidence for _ patterns inhibiting place-to-value coercions.

@RustyYato
Copy link

Like this?

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=3d2143d5f263fb1a6d324c65feb0dc31

fn main() {
    let x = (0, String::new());
    drop(x.1);
    let (x, _) = x;
    // let (x, y) = x; // doesn't compile
}

@RalfJung
Copy link
Member Author

RalfJung commented Dec 5, 2020

drop doesn't change the content of the location though, the value still adheres to the validity invariant (and it has to, otherwise ManuallyDrop would be unsound).

@scottmcm
Copy link
Member

scottmcm commented Dec 6, 2020

The following code is not even unsafe, so it certainly cannot be UB:

It was unsafe back in <=1.21, though. Is it not-unsafe on purpose, or by accident in the MIR unsafety check?

@tmiasko
Copy link

tmiasko commented Dec 6, 2020

The MIR for let _ = *p; is empty, so it could well be accidental.

@elichai
Copy link

elichai commented Dec 6, 2020

This is IMHO even more confusing because let _ = a; doesn't even drop a, so it feels to me like let _ = $expr doesn't discard the expression so the read should actually read.
(on the other hand you could argue that this is because $expr is just removed from MIR, and then it won't drop + it won't read, but that feels less intuitive for someone who doesn't think about IR when writing code)

@RalfJung
Copy link
Member Author

RalfJung commented Dec 6, 2020

let _ = a; doesn't even drop a

Well, it drops a sometimes...

fn mark() {}

fn test1(x: Vec<i32>) {
    let _ = x; // nothing happens here
    mark();
}

fn test2(x: Vec<i32>) {
    let _ = vec![1]; // this is dropped here
    mark();
}

@bjorn3
Copy link
Member

bjorn3 commented Dec 6, 2020

In test2, it isn't the _ that drops it. It is the fact that the temporary vec![1] goes out of scope that drops it.

@RalfJung
Copy link
Member Author

RalfJung commented Dec 6, 2020

Well, but the _ is what makes it "go out of scope", so I don't think one can entirely separate these two things.

One could just as well argue that let _ = x in test1 makes "x go out of scope".

@gThorondorsen
Copy link

One could just as well argue that let _ = x in test1 makes "x go out of scope".

Currently, it does not. But it does in this very similar test using let _ = {x}; instead.

@RalfJung
Copy link
Member Author

RalfJung commented Dec 6, 2020

Currently, it does not.

Yeah, it doesn't, but by analogy with let _ = vec![1];, maybe it should.

The current situation seems mostly consistent (except for rust-lang/rust#79735), but it's strange because behavior heavily depends on whether the expression on the right evaluate to a temporary or an existing place.

@Nadrieril
Copy link
Member

I guess it's for consistency with the fact that let (y, _) = x only moves out of x.0. In that context we don't want _ to move anything out

@RalfJung
Copy link
Member Author

@scottmcm also points out that there seems to be a difference between the statements let _ = *ptr; and *ptr;, which also seems odd at best.

@camelid
Copy link
Member

camelid commented Dec 28, 2020

This compiles effectively to let x = (*ptr).1, so again whatever data is in the second field does not matter. (Note that *ptr here occurs only as a place expression, not as a value expression, so it makes sense that in (*ptr).1 we do not require the second field to be valid.)

@RalfJung I think you mean (*ptr).0 here?

@RalfJung
Copy link
Member Author

Ah, yes... I am switching too often between languages that index tuples starting at 0 vs 1. ;)

@RalfJung
Copy link
Member Author

RalfJung commented Jan 8, 2021

@pnkfelix points out that adding a type annotations make a difference for the operational behavior of _ patterns... rust-lang/rust#80059 (comment).

@QuineDot
Copy link

As I understand things, Rust issue 10488 set the current semantics of let _ = ...; this comment in particular points out that the lack of binding means that the lifetime of the RHS is governed by the rules of temporaries.

@RalfJung
Copy link
Member Author

RalfJung commented Oct 7, 2022

Also Cc rust-lang/miri#2360, we probably want to consider some of the code around _ UB that is currently accepted by Miri.

@RalfJung
Copy link
Member Author

With rust-lang/rust#104844 and rust-lang/rust#103208 having landed, the issue is mostly resolved now: _ patterns construct a place but never load from it. That means let _ = place; is UB if and only if addr_of!(place) is UB, and same for the other ways of using _ patterns.

There's a bug where this is not quite true for the ! type: rust-lang/rust#117288. But I think that's just a bug, the semantic questions are all clear.

Do we have official docs anywhere on the semantics of _ patterns? That seems to be the last missing bit.

@RalfJung RalfJung added S-pending-documentation Status: The issue is "resolved," but this resolution needs documentation and removed C-open-question Category: An open question that we should revisit labels Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-validity Topic: Related to validity invariants S-pending-documentation Status: The issue is "resolved," but this resolution needs documentation
Projects
None yet
Development

No branches or pull requests

10 participants