Stop promoting all the things

[oli-obk]

fixes #91009

r? @RalfJung

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @RalfJung (or someone else) soon.

Please see the contribution instructions for more information.

[oli-obk]

@bors try (need to run crater and nominate for libs if crater is clean)

[bors]

⌛ Trying commit 9ec72df with merge 5dad09e4031c59e404e465a158e2077e61b9a860...

[bors]

☀️ Try build successful - checks-actions
Build commit: 5dad09e4031c59e404e465a158e2077e61b9a860 (5dad09e4031c59e404e465a158e2077e61b9a860)

[oli-obk]

@craterbot check

[craterbot]

👌 Experiment pr-105085 created and queued.
🤖 Automatically detected try build 5dad09e4031c59e404e465a158e2077e61b9a860
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🚧 Experiment pr-105085 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[RalfJung]

Could you also add this non-const example from the issue? https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=3291dde278a238c06ce81db3decfb294

[RalfJung]

That is not the same test. Note that the one I linked did a bunch of stuff to register functions as rustc_promotable which is important. staged_api is required, too, otherwise rustc_promotable has no effect.

[RalfJung]

And here is another testcase :)

[RalfJung]

Actually we can make that a run-pass test instead -- might be good to have one like that as well:

use std::sync::atomic::*;

static FLAG: AtomicBool = AtomicBool::new(false);

struct NoisyDrop(&'static str);
impl Drop for NoisyDrop {
    fn drop(&mut self) {
        FLAG.store(true, Ordering::SeqCst);
    }
}
fn main() {
    {
        let _val = &&(NoisyDrop("drop!"), 0).1;
    }
    assert!(FLAG.load(Ordering::SeqCst));
}

[RalfJung]

r=me on code and tests, let's see what crater says.

Will this need t-lang FCP?

[oli-obk]

Will this need t-lang FCP?

yes, but I won't nominate it until we have the crater result.

[craterbot]

🎉 Experiment pr-105085 is completed!
📊 35 regressed and 15 fixed (249331 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[oli-obk]

The only regression is https://github.com/slightlyoutofphase/staticvec/blob/64c0a85c04b2ffc27ea3b20b87097f0dc49968f3/src/lib.rs#L31 which uses so many unstable features I think we can safely break them. cc @slightlyoutofphase

[oli-obk]

Nominating for T-lang:

This PR is a breaking change fixing a hole in our promotion checks. Most notably:

struct NoisyDrop(&'static str);
impl Drop for NoisyDrop {
    fn drop(&mut self) {
        println!("{}", self.0);
    }
}
fn main() {
    let _ = &&(NoisyDrop("drop!"), 0).1;
    // NosiyDrop is not dropped ...
}

does not drop the NoisyDrop, as it gets implicitly promoted. This is obviously a bug, as the user did not ask for promotion and we should not silently promote away side effects like dropping.

Crater has found a single breaking change in a nightly-only crate.

@rust-lang/wg-const-eval would like to merge this PR.

[rfcbot]

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

• @cramertj
• @joshtriplett
• @nikomatsakis
• @pnkfelix
• @scottmcm

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[nikomatsakis]

@rfcbot reviewed

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[oli-obk]

@bors r=RalfJung

[bors]

📌 Commit b4a6047 has been approved by RalfJung

It is now in the queue for this repository.

[bors]

⌛ Testing commit b4a6047 with merge f5c3dfd...

[bors]

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing f5c3dfd to master...

[rust-timer]

Finished benchmarking commit (f5c3dfd): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.9%	[0.8%, 1.0%]	3
Regressions ❌ (secondary)	1.6%	[0.9%, 4.1%]	6
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.9%	[0.8%, 1.0%]	3

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.0%	[2.0%, 2.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

[pnkfelix]

• The three primary regressions are all to unicode-normalization {check-full, debug-full, check-incr-full}, by 1% or less.
• The secondary regressions are to coercions-debug-full (by 4%), and to variations of ucd (by 1.25% or less).
• In my judgement, the semantic problem this addresses more than offsets the cost that this PR is paying. Marking as triaged.

@rustbot label: +perf-regression-triaged