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

apint fails to compile on beta #55806

Closed
Mark-Simulacrum opened this issue Nov 9, 2018 · 12 comments · Fixed by #55828
Closed

apint fails to compile on beta #55806

Mark-Simulacrum opened this issue Nov 9, 2018 · 12 comments · Fixed by #55828
Assignees
Labels
P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone

Comments

@Mark-Simulacrum
Copy link
Member

apint fails to compile (https://crater-reports.s3.amazonaws.com/beta-1.31-1/beta-2018-10-30/reg/apint-0.2.0/log.txt) on beta. Note that the crate must be compiled in test mode.

Nov 03 02:59:10.198 INFO kablam!    Compiling apint v0.2.0 (/source)
Nov 03 02:59:13.651 INFO kablam! error[E0597]: borrowed value does not live long enough
Nov 03 02:59:13.651 INFO kablam!    --> src/apint/constructors.rs:429:11
Nov 03 02:59:13.651 INFO kablam!     |
Nov 03 02:59:13.651 INFO kablam! 429 |               .chain([
Nov 03 02:59:13.651 INFO kablam!     |  ____________________^
Nov 03 02:59:13.651 INFO kablam! 430 | |                 u8::max_value(),
Nov 03 02:59:13.651 INFO kablam! 431 | |                 10,
Nov 03 02:59:13.651 INFO kablam! 432 | |                 42,
Nov 03 02:59:13.651 INFO kablam! 433 | |                 99,
Nov 03 02:59:13.651 INFO kablam! 434 | |                 123
Nov 03 02:59:13.651 INFO kablam! 435 | |             ].into_iter()
Nov 03 02:59:13.651 INFO kablam!     | |_____________^ temporary value does not live long enough
Nov 03 02:59:13.651 INFO kablam! 436 |                .map(|v| *v))
Nov 03 02:59:13.651 INFO kablam! 437 |       }
Nov 03 02:59:13.651 INFO kablam!     |       - temporary value only lives until here
Nov 03 02:59:13.651 INFO kablam!     |
Nov 03 02:59:13.651 INFO kablam!     = note: borrowed value must be valid for the static lifetime...
Nov 03 02:59:13.651 INFO kablam! 
Nov 03 02:59:13.657 INFO kablam! error[E0597]: borrowed value does not live long enough
Nov 03 02:59:13.657 INFO kablam!    --> src/apint/constructors.rs:480:11
Nov 03 02:59:13.657 INFO kablam!     |
Nov 03 02:59:13.657 INFO kablam! 480 |               .chain([
Nov 03 02:59:13.657 INFO kablam!     |  ____________________^
Nov 03 02:59:13.657 INFO kablam! 481 | |                 u16::max_value(),
Nov 03 02:59:13.657 INFO kablam! 482 | |                 500,
Nov 03 02:59:13.657 INFO kablam! 483 | |                 1000,
Nov 03 02:59:13.657 INFO kablam! ...   |
Nov 03 02:59:13.657 INFO kablam! 486 | |                 42_000
Nov 03 02:59:13.657 INFO kablam! 487 | |             ].into_iter().map(|v| *v))
Nov 03 02:59:13.657 INFO kablam!     | |_____________^ temporary value does not live long enough
Nov 03 02:59:13.657 INFO kablam! 488 |       }
Nov 03 02:59:13.657 INFO kablam!     |       - temporary value only lives until here
Nov 03 02:59:13.657 INFO kablam!     |
Nov 03 02:59:13.657 INFO kablam!     = note: borrowed value must be valid for the static lifetime...
Nov 03 02:59:13.657 INFO kablam! 
Nov 03 02:59:13.658 INFO kablam! error[E0597]: borrowed value does not live long enough
Nov 03 02:59:13.658 INFO kablam!    --> src/apint/constructors.rs:513:11
Nov 03 02:59:13.658 INFO kablam!     |
Nov 03 02:59:13.658 INFO kablam! 513 |               .chain([
Nov 03 02:59:13.658 INFO kablam!     |  ____________________^
Nov 03 02:59:13.658 INFO kablam! 514 | |                 u32::max_value(),
Nov 03 02:59:13.658 INFO kablam! 515 | |                 1_000_000,
Nov 03 02:59:13.658 INFO kablam! 516 | |                 999_999_999,
Nov 03 02:59:13.658 INFO kablam! 517 | |                 1_234_567_890
Nov 03 02:59:13.658 INFO kablam! 518 | |             ].into_iter().map(|v| *v))
Nov 03 02:59:13.658 INFO kablam!     | |_____________^ temporary value does not live long enough
Nov 03 02:59:13.658 INFO kablam! 519 |       }
Nov 03 02:59:13.658 INFO kablam!     |       - temporary value only lives until here
Nov 03 02:59:13.659 INFO kablam!     |
Nov 03 02:59:13.659 INFO kablam!     = note: borrowed value must be valid for the static lifetime...
Nov 03 02:59:13.659 INFO kablam! 
Nov 03 02:59:13.663 INFO kablam! error[E0597]: borrowed value does not live long enough
Nov 03 02:59:13.663 INFO kablam!    --> src/apint/constructors.rs:544:11
Nov 03 02:59:13.663 INFO kablam!     |
Nov 03 02:59:13.663 INFO kablam! 544 |               .chain([
Nov 03 02:59:13.663 INFO kablam!     |  ____________________^
Nov 03 02:59:13.663 INFO kablam! 545 | |                 u64::max_value(),
Nov 03 02:59:13.663 INFO kablam! 546 | |                 1_000_000_000_000,
Nov 03 02:59:13.663 INFO kablam! 547 | |                 999_999_999_999_999_999,
Nov 03 02:59:13.663 INFO kablam! 548 | |                 0x0123_4567_89AB_CDEF
Nov 03 02:59:13.663 INFO kablam! 549 | |             ].into_iter().map(|v| *v))
Nov 03 02:59:13.663 INFO kablam!     | |_____________^ temporary value does not live long enough
Nov 03 02:59:13.663 INFO kablam! 550 |       }
Nov 03 02:59:13.664 INFO kablam!     |       - temporary value only lives until here
Nov 03 02:59:13.664 INFO kablam!     |
Nov 03 02:59:13.664 INFO kablam!     = note: borrowed value must be valid for the static lifetime...
Nov 03 02:59:13.664 INFO kablam! 
Nov 03 02:59:13.671 INFO kablam! error[E0597]: borrowed value does not live long enough
Nov 03 02:59:13.671 INFO kablam!    --> src/apint/constructors.rs:575:11
Nov 03 02:59:13.671 INFO kablam!     |
Nov 03 02:59:13.671 INFO kablam! 575 |               .chain([
Nov 03 02:59:13.671 INFO kablam!     |  ____________________^
Nov 03 02:59:13.671 INFO kablam! 576 | |                 u128::max_value(),
Nov 03 02:59:13.671 INFO kablam! 577 | |                 1_000_000_000_000_000_000_000_000,
Nov 03 02:59:13.671 INFO kablam! 578 | |                 999_999_999_999_999_999_999_999_999,
Nov 03 02:59:13.671 INFO kablam! 579 | |                 0x0123_4567_89AB_CDEF_FEDC_BA98_7654_3210
Nov 03 02:59:13.671 INFO kablam! 580 | |             ].into_iter().map(|v| *v))
Nov 03 02:59:13.671 INFO kablam!     | |_____________^ temporary value does not live long enough
Nov 03 02:59:13.672 INFO kablam! 581 |       }
Nov 03 02:59:13.672 INFO kablam!     |       - temporary value only lives until here
Nov 03 02:59:13.672 INFO kablam!     |
Nov 03 02:59:13.672 INFO kablam!     = note: borrowed value must be valid for the static lifetime...
Nov 03 02:59:13.672 INFO kablam! 
Nov 03 02:59:14.105 INFO kablam! error: aborting due to 5 previous errors
@Mark-Simulacrum Mark-Simulacrum added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Nov 9, 2018
@Mark-Simulacrum Mark-Simulacrum added this to the 1.31 milestone Nov 9, 2018
@Mark-Simulacrum
Copy link
Member Author

cc @Robbepop -- I believe this is your crate, though it seems plausible this is a bug in the compiler

@Mark-Simulacrum
Copy link
Member Author

cc @rust-lang/compiler -- beta/stable regression, would be good to proactively triage before meeting if possible

@pnkfelix pnkfelix self-assigned this Nov 9, 2018
@eddyb
Copy link
Member

eddyb commented Nov 9, 2018

@oli-obk Could this be promotion-related?

@oli-obk
Copy link
Contributor

oli-obk commented Nov 9, 2018

Yea. The call to into_iter on arrays does not actually do what it advertises, it derefs to a slice and then does into_iter. We stopped allowing const fns in promoteds #53851. max_value is still marked as promotable, so that shouldn't be the issue. I'll investigate later today when I get back to a PC

@Robbepop
Copy link
Contributor

Robbepop commented Nov 9, 2018

@Mark-Simulacrum yeah that's my crate thanks for linking!

I have already stumbled upon this CI error but couldn't yet find time to wrap my head around it.
Though I thought it was a bug in a former version of the borrow checker that now got fixed.

Yea. The call to into_iter on arrays does not actually do what it advertises, it derefs to a slice and then does into_iter. We stopped allowing const fns in promoteds #53851. max_value is still marked as promotable, so that shouldn't be the issue. I'll investigate later today when I get back to a PC

So if I understood correctly simply using a vec![..] instead of plain [..] would solve that issue?
Maybe we should reiterate on this design for into_iter on arrays if that's the case. :D

@oli-obk
Copy link
Contributor

oli-obk commented Nov 9, 2018

minimal repro

    let x: &'static _ = &u8::max_value();

Though I thought it was a bug in a former version of the borrow checker that now got fixed.

Nope, it's a new bug that I was sure we had regression tests against.

@pnkfelix
Copy link
Member

pnkfelix commented Nov 9, 2018

This was injected between rustc 1.31.0-nightly (5597ee8 2018-10-03) and rustc 1.31.0-nightly (8c4ad4e 2018-10-04)

Log of bors commits in that time in the details block

% git log 5597ee8..8c4ad4e --author bors --format=oneline
8c4ad4e Auto merge of #54649 - nikomatsakis:universes-refactor-1, r=scalexm
a57f1c9 Auto merge of #54666 - matthewjasper:mir-function-spans, r=pnkfelix
5de5281 Auto merge of #54784 - Manishearth:clippyup, r=oli-obk
5472b07 Auto merge of #54809 - pietroalbini:rollup, r=pietroalbini
8a0e5cb Auto merge of #54638 - christianpoveda:master, r=kennytm
088fc73 Auto merge of #53851 - oli-obk:local_promotion, r=eddyb
c67ea54 Auto merge of #54624 - arielb1:evaluate-outlives, r=nikomatsakis
d078728 Auto merge of #54447 - KiChjang:issue-54331, r=nikomatsakis
4bf883b Auto merge of #54391 - davidtwco:issue-54230, r=petrochenkov

@pnkfelix
Copy link
Member

pnkfelix commented Nov 9, 2018

Hmm I wonder if this could somehow be a consequence fo #54447 ...?

  • update: probably not, since this afflicts both AST-borrowck and MIR-borrowck.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 9, 2018

Full diff: 5597ee8...8c4ad4e

#53851 (limiting promotability of const fns) also fell in there.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 9, 2018

I even added unit tests, but never any for promoting functions from the libstd.

fn is_promotable_const_fn<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> bool {
will only work if a function call is promoted in the same crate where the rustc_promotable function is declared.

I think the solution is to make the const qualifier eagerly call is_promotable_const_fn while a function is qualified to cache the result (in the query engine), so that the query is never evaluated in the perspective of another crate (and add an assert!(def_id.is_local())) to is_promotable_const_fn to safeguard against messing this up

@pnkfelix pnkfelix added the P-high High priority label Nov 9, 2018
@pnkfelix
Copy link
Member

pnkfelix commented Nov 9, 2018

Discussed a bit with @oli-obk and investigated independently. Some details of note:

  • The implementation of #[rustc_promotable] relies on #![feature(staged_api)]. If you don't have the latter, the former will essentially be silently ignored, AFAICT.
  • It seems like there are some holes in @oli-obk 's analysis above. In particular, I attempted to make a unit test with multiple crates, and it seemed like promotion was still happening. See here:

% cat ../../issue_55806_lib.rs

#![stable(feature = "unit_test", since="1.0.0")]
#![feature(staged_api, rustc_attrs)]
#![crate_type="lib"]

#[stable(feature = "unit_test", since="1.0.0")]
pub struct S(u8);

#[stable(feature = "unit_test", since="1.0.0")]
#[rustc_promotable]
pub const fn bar() -> S { S(10) }

% cat ../../issue-55806-xc.rs

extern crate issue_55806_lib;

use issue_55806_lib::bar;

fn main() {
    let _x: &'static _ = &bar();
}
% ./build/x86_64-unknown-linux-gnu/stage1/bin/rustc ../../issue_55806_lib.rs && ./build/x86_64-unknown-linux-gnu/stage1/bin/rustc ../../issue-55806-xc.rs --extern issue_55806_lib=./libissue_55806_lib.rlib
%

@pnkfelix
Copy link
Member

pnkfelix commented Nov 9, 2018

reassigning to @oli-obk for followup investigation and hopefully fixing.

@pnkfelix pnkfelix assigned oli-obk and unassigned pnkfelix Nov 9, 2018
pietroalbini added a commit to pietroalbini/rust that referenced this issue Nov 10, 2018
…eddyb

Add missing `rustc_promotable` attribute to unsigned `min_value` and `max_value`

cc @pnkfelix

fixes rust-lang#55806
bors added a commit that referenced this issue Nov 12, 2018
…, r=estebank

[beta] backport rollup (typeck match arms eagerly; add rustc_promotable to unsigneds)

Backport of 5f91373 from (beta-accepted) PR #55819

Fix #55810
Fix #55806
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants