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

Post-drop elaboration const-checking fails on ZSTs #90770

Closed
ecstatic-morse opened this issue Nov 10, 2021 · 2 comments · Fixed by #91410
Closed

Post-drop elaboration const-checking fails on ZSTs #90770

ecstatic-morse opened this issue Nov 10, 2021 · 2 comments · Fixed by #91410
Assignees
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) B-unstable Blocker: Implemented in the nightly compiler and unstable. C-bug Category: This is a bug.

Comments

@ecstatic-morse
Copy link
Contributor

ecstatic-morse commented Nov 10, 2021

Found by @nbdd0121 here, the following code compiles when it shouldn't:

#![feature(const_precise_live_drops)]

struct S;

impl Drop for S {
    fn drop(&mut self) {
        println!("Hello!");
    }
}

const fn foo() {
    let s = S;
}

fn main() {}

The final "post-borrowck cleanup" pass is the Deaggregator, which replaces the assignment _1 = S; with a nop because S has no fields. That means _1 never gets initialized in the MIR before its Drop terminator, so the qualification logic doesn't assign it NeedsDrop.

@ecstatic-morse ecstatic-morse self-assigned this Nov 10, 2021
@ecstatic-morse ecstatic-morse added A-const-eval Area: Constant evaluation (MIR interpretation) B-unstable Blocker: Implemented in the nightly compiler and unstable. C-bug Category: This is a bug. labels Nov 10, 2021
@ecstatic-morse
Copy link
Contributor Author

Deaggregator was moved into post-borrowck cleanup from optimizations in #73656, cc @oli-obk @wesleywiser. I think we need to come up with a more robust system for splitting "MIR for analysis" and "MIR for optimization", the current system places an extremely high burden on contributors and reviewers. A simple reordering of passes can have unforeseen consequences.

@RalfJung
Copy link
Member

We should also document more clearly what our analyses need from MIR, so that one can determine if a change in "MIR for analysis" can break our analyses or not.

ecstatic-morse added a commit to ecstatic-morse/rust that referenced this issue Dec 1, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 2, 2021
…rops-take-2, r=oli-obk

Move `#![feature(const_precise_live_drops)]` checks earlier in the pipeline

Should mitigate the issues found during MCP on rust-lang#73255.

Once this is done, we should clean up the queries a bit, since I think `mir_drops_elaborated_and_const_checked` can be merged back into `mir_promoted`.

Fixes rust-lang#90770.

cc `@rust-lang/wg-const-eval`
r? `@nikomatsakis` (since they reviewed rust-lang#71824)
@bors bors closed this as completed in 3964131 Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) B-unstable Blocker: Implemented in the nightly compiler and unstable. C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants