de-promote Duration::from_secs #71796

RalfJung · 2020-05-02T10:20:33Z

In #67531, we removed the rustc_promotable attribute from a bunch of Duration methods, but not from Duration::from_secs. This makes the current list of promotable functions the following (courtesy of @ecstatic-morse):

INT::min_value, INT::max_value
std::mem::size_of, std::mem::align_of
RangeInclusive::new (backing x..=y)
std::ptr::null, std::ptr::null_mut
RawWaker::new, RawWakerVTable::new ???
Duration::from_secs

I feel like the last one stands out a bit here -- the rest are all very core language primitives, and RawWaker has a strong motivation for getting a 'static vtable. But a &'static Duration? That seems unlikely. So I propose we no longer promote calls to Duration::from_secs, which is what this PR does.

#67531 saw zero regressions and I am not aware of anyone complaining that this broke their (non-cratered) code, so I consider it likely the same will be true here, but of course we'd do a crater run.

See this document for some more background on promotion and rust-lang/const-eval#19 for some of the concerns around promoting function calls.

rust-highfive · 2020-05-02T10:20:37Z

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

nikomatsakis · 2020-05-02T10:24:47Z

@bors try

bors · 2020-05-02T10:24:58Z

⌛ Trying commit 58ae4a9 with merge b2a885c600d942fb829a214e798f7ae205a0594e...

bors · 2020-05-02T12:57:55Z

☀️ Try build successful - checks-azure
Build commit: b2a885c600d942fb829a214e798f7ae205a0594e (b2a885c600d942fb829a214e798f7ae205a0594e)

RalfJung · 2020-05-04T09:33:09Z

@nikomatsakis so do you agree this is worthwhile and we should try to crater it?

nikomatsakis · 2020-05-04T20:16:04Z

I do, which is why I did the bors try command.

@craterbot check

craterbot · 2020-05-04T20:16:10Z

👌 Experiment pr-71796 created and queued.
🤖 Automatically detected try build b2a885c600d942fb829a214e798f7ae205a0594e
🔍 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 · 2020-05-16T02:06:11Z

🚧 Experiment pr-71796 is now running

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

pietroalbini · 2020-05-17T22:39:00Z

@craterbot p=1

craterbot · 2020-05-17T22:39:01Z

🚨 Error: it's only possible to edit queued experiments

🆘 If you have any trouble with Crater please ping @rust-lang/infra!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2020-05-21T14:50:20Z

🎉 Experiment pr-71796 is completed!
📊 2 regressed and 1 fixed (102844 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

RalfJung · 2020-05-21T17:27:04Z

Both regressions are spurious. So, looks like on the part of the ecosystem that crater covers, this will not break anything.

@nikomatsakis what is the next step here?

Mark-Simulacrum · 2020-05-21T18:42:13Z

With my release team hat on I'd say the zero regressions from the crater do look quite promising. I would suggest that we land this at the start of a nightly cycle, though, so ~June 4th -- in a few weeks -- just to give it as much time to bake as possible.

nikomatsakis · 2020-05-21T21:25:31Z

r=me but waiting until the start of the cycle seems fine.

@rust-lang/lang -- I'm not going to bother nominating this as I don't anticipate controversy. The proposal here is to remove Duration::from_secs from the list of functions that can be implicitly promoted into a constant. Crater tests found no regressions.

joshtriplett · 2020-05-22T02:58:03Z

But a &'static Duration? That seems unlikely.

Would this still allow const { Duration::from_secs(1) } once that syntax exists? Does this just prevent automatically promoting a constant Duration?

RalfJung · 2020-05-22T06:56:31Z

@joshtriplett yes and yes.

joshtriplett · 2020-05-22T18:14:23Z

No objections then.

@ecstatic-morse

de-promote Duration::from_secs In rust-lang#67531, we removed the `rustc_promotable` attribute from a bunch of `Duration` methods, but not from `Duration::from_secs`. This makes the current list of promotable functions the following (courtesy of @ecstatic-morse): * `INT::min_value`, `INT::max_value` * `std::mem::size_of`, `std::mem::align_of` * `RangeInclusive::new` (backing `x..=y`) * `std::ptr::null`, `std::ptr::null_mut` * `RawWaker::new`, `RawWakerVTable::new` ??? * `Duration::from_secs` I feel like the last one stands out a bit here -- the rest are all very core language primitives, and `RawWaker` has a strong motivation for getting a `'static` vtable. But a `&'static Duration`? That seems unlikely. So I propose we no longer promote calls to `Duration::from_secs`, which is what this PR does. rust-lang#67531 saw zero regressions and I am not aware of anyone complaining that this broke their (non-cratered) code, so I consider it likely the same will be true here, but of course we'd do a crater run. See [this document](https://github.com/rust-lang/const-eval/blob/master/promotion.md) for some more background on promotion and rust-lang/const-eval#19 for some of the concerns around promoting function calls.

@ecstatic-morse

de-promote Duration::from_secs In rust-lang#67531, we removed the `rustc_promotable` attribute from a bunch of `Duration` methods, but not from `Duration::from_secs`. This makes the current list of promotable functions the following (courtesy of @ecstatic-morse): * `INT::min_value`, `INT::max_value` * `std::mem::size_of`, `std::mem::align_of` * `RangeInclusive::new` (backing `x..=y`) * `std::ptr::null`, `std::ptr::null_mut` * `RawWaker::new`, `RawWakerVTable::new` ??? * `Duration::from_secs` I feel like the last one stands out a bit here -- the rest are all very core language primitives, and `RawWaker` has a strong motivation for getting a `'static` vtable. But a `&'static Duration`? That seems unlikely. So I propose we no longer promote calls to `Duration::from_secs`, which is what this PR does. rust-lang#67531 saw zero regressions and I am not aware of anyone complaining that this broke their (non-cratered) code, so I consider it likely the same will be true here, but of course we'd do a crater run. See [this document](https://github.com/rust-lang/const-eval/blob/master/promotion.md) for some more background on promotion and rust-lang/const-eval#19 for some of the concerns around promoting function calls.

bors · 2020-06-06T14:35:02Z

⌛ Testing commit 58ae4a9 with merge 23ff1657a24783bddd42e51cfa70902060439a9d...

@ecstatic-morse

de-promote Duration::from_secs In rust-lang#67531, we removed the `rustc_promotable` attribute from a bunch of `Duration` methods, but not from `Duration::from_secs`. This makes the current list of promotable functions the following (courtesy of @ecstatic-morse): * `INT::min_value`, `INT::max_value` * `std::mem::size_of`, `std::mem::align_of` * `RangeInclusive::new` (backing `x..=y`) * `std::ptr::null`, `std::ptr::null_mut` * `RawWaker::new`, `RawWakerVTable::new` ??? * `Duration::from_secs` I feel like the last one stands out a bit here -- the rest are all very core language primitives, and `RawWaker` has a strong motivation for getting a `'static` vtable. But a `&'static Duration`? That seems unlikely. So I propose we no longer promote calls to `Duration::from_secs`, which is what this PR does. rust-lang#67531 saw zero regressions and I am not aware of anyone complaining that this broke their (non-cratered) code, so I consider it likely the same will be true here, but of course we'd do a crater run. See [this document](https://github.com/rust-lang/const-eval/blob/master/promotion.md) for some more background on promotion and rust-lang/const-eval#19 for some of the concerns around promoting function calls.

RalfJung · 2020-06-06T14:47:29Z

yield
@bors retry

bors · 2020-06-06T16:49:02Z

⌛ Testing commit 58ae4a9 with merge 6543d83271ab6a1d8c1553a1ddb183e604a96ebd...

@ecstatic-morse

de-promote Duration::from_secs In rust-lang#67531, we removed the `rustc_promotable` attribute from a bunch of `Duration` methods, but not from `Duration::from_secs`. This makes the current list of promotable functions the following (courtesy of @ecstatic-morse): * `INT::min_value`, `INT::max_value` * `std::mem::size_of`, `std::mem::align_of` * `RangeInclusive::new` (backing `x..=y`) * `std::ptr::null`, `std::ptr::null_mut` * `RawWaker::new`, `RawWakerVTable::new` ??? * `Duration::from_secs` I feel like the last one stands out a bit here -- the rest are all very core language primitives, and `RawWaker` has a strong motivation for getting a `'static` vtable. But a `&'static Duration`? That seems unlikely. So I propose we no longer promote calls to `Duration::from_secs`, which is what this PR does. rust-lang#67531 saw zero regressions and I am not aware of anyone complaining that this broke their (non-cratered) code, so I consider it likely the same will be true here, but of course we'd do a crater run. See [this document](https://github.com/rust-lang/const-eval/blob/master/promotion.md) for some more background on promotion and rust-lang/const-eval#19 for some of the concerns around promoting function calls.

RalfJung · 2020-06-06T17:06:14Z

@bors retry

bors · 2020-06-06T19:30:58Z

⌛ Testing commit 58ae4a9 with merge 42cda8ef3fabb5418ef717b08c7f056188ad4738...

RalfJung · 2020-06-06T19:58:22Z

@bors retry rollup=always

@ghost

Rollup of 3 pull requests Successful merges: - rust-lang#71796 (de-promote Duration::from_secs) - rust-lang#72508 (Make `PolyTraitRef::self_ty` return `Binder<Ty>`) - rust-lang#72708 (linker: Add a linker rerun hack for gcc versions not supporting -static-pie) Failed merges: r? @ghost

flip1995 · 2020-06-07T01:06:40Z

@Mark-Simulacrum this was merged, even though it breaks Clippys UI tests: https://dev.azure.com/rust-lang/rust/_build/results?buildId=31276&view=logs&j=e50ab380-190b-535c-8a18-25c988f7577b&t=4a9c8583-1de4-50df-1657-629e71b187fc&l=7899

Admittedly one Clippy UI test is broken in the rustc testsuite anyway (something, something, proc_macro), but I thought with #72674 this shouldn't happen anymore.

Mark-Simulacrum · 2020-06-07T01:12:13Z

Hm yeah I would've expected that to not happen to. I'll try to take a look tomorrow.

Mark-Simulacrum · 2020-06-07T14:08:41Z

Fixed in #73097

de-promote Duration::from_secs

58ae4a9

rust-highfive assigned nikomatsakis May 2, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 2, 2020

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 4, 2020

ecstatic-morse mentioned this pull request May 5, 2020

Inline const expressions and patterns rust-lang/rfcs#2920

Merged

crlf0710 added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels May 15, 2020

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels May 21, 2020

nikomatsakis added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 22, 2020

RalfJung mentioned this pull request Jun 6, 2020

Rollup of 7 pull requests #73052

Closed

RalfJung mentioned this pull request Jun 6, 2020

Rollup of 7 pull requests #73053

Closed

RalfJung mentioned this pull request Jun 6, 2020

Rollup of 7 pull requests #73062

Closed

RalfJung mentioned this pull request Jun 6, 2020

Rollup of 9 pull requests #73064

Closed

RalfJung mentioned this pull request Jun 6, 2020

Rollup of 3 pull requests #73073

Merged

bors merged commit 64c27f9 into rust-lang:master Jun 7, 2020

flip1995 added a commit to flip1995/rust-clippy that referenced this pull request Jun 7, 2020

Rustup to rust-lang/rust#71796

5bdbc45

RalfJung deleted the from-secs branch June 7, 2020 08:00

RalfJung mentioned this pull request Aug 12, 2020

Solving the problem with promotion rust-lang/const-eval#53

Closed

Aaron1011 mentioned this pull request Dec 29, 2020

WIP crater experiment: do not promote const fn calls implicitly ever #80243

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

de-promote Duration::from_secs #71796

de-promote Duration::from_secs #71796

RalfJung commented May 2, 2020

rust-highfive commented May 2, 2020

nikomatsakis commented May 2, 2020

bors commented May 2, 2020

bors commented May 2, 2020

RalfJung commented May 4, 2020

nikomatsakis commented May 4, 2020

craterbot commented May 4, 2020

craterbot commented May 16, 2020

pietroalbini commented May 17, 2020

craterbot commented May 17, 2020

craterbot commented May 21, 2020

RalfJung commented May 21, 2020

Mark-Simulacrum commented May 21, 2020

nikomatsakis commented May 21, 2020 •

edited

Loading

joshtriplett commented May 22, 2020

RalfJung commented May 22, 2020

joshtriplett commented May 22, 2020

bors commented Jun 6, 2020

RalfJung commented Jun 6, 2020

bors commented Jun 6, 2020

RalfJung commented Jun 6, 2020

bors commented Jun 6, 2020

RalfJung commented Jun 6, 2020

flip1995 commented Jun 7, 2020

Mark-Simulacrum commented Jun 7, 2020

Mark-Simulacrum commented Jun 7, 2020

de-promote Duration::from_secs #71796

de-promote Duration::from_secs #71796

Conversation

RalfJung commented May 2, 2020

rust-highfive commented May 2, 2020

nikomatsakis commented May 2, 2020

bors commented May 2, 2020

bors commented May 2, 2020

RalfJung commented May 4, 2020

nikomatsakis commented May 4, 2020

craterbot commented May 4, 2020

craterbot commented May 16, 2020

pietroalbini commented May 17, 2020

craterbot commented May 17, 2020

craterbot commented May 21, 2020

RalfJung commented May 21, 2020

Mark-Simulacrum commented May 21, 2020

nikomatsakis commented May 21, 2020 • edited Loading

joshtriplett commented May 22, 2020

RalfJung commented May 22, 2020

joshtriplett commented May 22, 2020

bors commented Jun 6, 2020

RalfJung commented Jun 6, 2020

bors commented Jun 6, 2020

RalfJung commented Jun 6, 2020

bors commented Jun 6, 2020

RalfJung commented Jun 6, 2020

flip1995 commented Jun 7, 2020

Mark-Simulacrum commented Jun 7, 2020

Mark-Simulacrum commented Jun 7, 2020

nikomatsakis commented May 21, 2020 •

edited

Loading