Skip to content

Conversation

dianqk
Copy link
Member

@dianqk dianqk commented Oct 19, 2025

Fixes #141313 and #141313 (comment).

I agree with what #147844 (comment) mentioned:

We can reason with the value behind a reference because it is UB to directly assign to the underlying local while the reference is live. We allow creating new derefs, this means extending the liveness of references, so we are creating UB.

I don't think we should ban introducing new derefs, since this may prevent many optimizations.

r? cjgillot

@rustbot
Copy link
Collaborator

rustbot commented Oct 19, 2025

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 19, 2025
/// Check that we allow dereferences in the RHS if the LHS is a stable projection.
// This introduces copy overlapping if dereferencing `_2` or `_4`.
#[custom_mir(dialect = "runtime")]
fn stable_projection(_1: (Adt,)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@cjgillot I just found this still introduces copy overlapping.

@dianqk
Copy link
Member Author

dianqk commented Oct 19, 2025

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

rust-bors bot added a commit that referenced this pull request Oct 19, 2025
GVN: Only introduce new derefs if the immutable borrow is always valid.
@rust-bors

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 19, 2025
@cjgillot
Copy link
Contributor

I'd have kept introducing Value::Projection(Deref) as exists, but forbidden introducing Deref projections in try_as_place. What do you think?

@rust-bors
Copy link

rust-bors bot commented Oct 19, 2025

☀️ Try build successful (CI)
Build commit: efb5f98 (efb5f98020c7392070aae01c719ed60b78144ff5, parent: c6efb9019b3169fc672248339dbbf13e6a134de3)

@rust-timer
Copy link
Collaborator

Queued efb5f98 with parent c6efb90, future comparison URL.
There are currently 0 preceding artifacts in the queue.
It will probably take at least ~1.1 hours until the benchmark run finishes.

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

Labels

S-waiting-on-perf Status: Waiting on a perf run to be completed. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GVN misunderstands aliasing, can create overlapping assignments (again)

4 participants