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

Type annotations needed error on definition site #84398

Closed
vorner opened this issue Apr 21, 2021 · 9 comments · Fixed by #84559
Closed

Type annotations needed error on definition site #84398

vorner opened this issue Apr 21, 2021 · 9 comments · Fixed by #84559
Labels
C-bug Category: This is a bug. 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

@vorner
Copy link
Contributor

vorner commented Apr 21, 2021

Hello

After upgrading nightly rustc, I'm getting a lot of type inference errors on code that was compiling fine previously. Moreover, these are not at the call site, but at the definition site, so there doesn't seem to be any place to put any type annotations, like this:

    --> src/spirit.rs:996:1
     |
996  | / impl<O, C> SpiritBuilder for Builder<O, C>
997  | | where
998  | |     Self::Config: DeserializeOwned + Send + Sync + 'static,
999  | |     Self::Opts: StructOpt + Sync + Send + 'static,
...    |
1108 | |     }
1109 | | }
     | |_^ cannot infer type for type parameter `C`

It spills more kinds of errors that make little sense, but I have the impression these will have a common cause:

error[E0277]: `C` cannot be shared between threads safely
   --> src/spirit.rs:998:45
    |
998 |     Self::Config: DeserializeOwned + Send + Sync + 'static,
    |                                             ^^^^ `C` cannot be shared between threads safely

The problematic code is both released on crates.io (crate spirit, version 0.4.16, but I expect that to be happening through most of the history of the crate), and in master (vorner/spirit@1495c1c).

I think this is a regression, as it was compiling previously.

It compiles fine with older nightly (and stable is fine way back, this particular code is looking mostly the same for some 2 years or more):

rustc 1.53.0-nightly (07e0e2ec2 2021-03-24)

Breaks with

rustc 1.53.0-nightly (6df26f897 2021-04-20)
binary: rustc
commit-hash: 6df26f897cffb2d86880544bb451c6b5f8509b2d
commit-date: 2021-04-20
host: x86_64-unknown-linux-gnu
release: 1.53.0-nightly
LLVM version: 12.0.0

(Unfortunately, I haven't been keeping fully up to date recently, so I don't have a smaller range).

@vorner vorner added the C-bug Category: This is a bug. label Apr 21, 2021
@jonas-schievink jonas-schievink added the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label Apr 21, 2021
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Apr 21, 2021
@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 21, 2021
@hameerabbasi
Copy link
Contributor

Bisected:

searched nightlies: from nightly-2021-01-01 to nightly-2021-04-20
regressed nightly: nightly-2021-04-01
searched commits: from 74874a6 to 4fdac23
regressed commit: 4fdac23

bisected with cargo-bisect-rustc v0.6.0

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --test-dir=. --start=2021-01-01

Regressed PR is #76814, cc @jackh726.

@jackh726
Copy link
Member

jackh726 commented Apr 21, 2021

An MCVE would be extremely helpful here.

@jackh726 jackh726 added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Apr 21, 2021
@hameerabbasi
Copy link
Contributor

@rustbot ping icebreakers-cleanup-crew

I've tried without success to reduce this, it's buried in layers of crates. Maybe one of you will have better luck.

@rustbot
Copy link
Collaborator

rustbot commented Apr 21, 2021

@rustbot rustbot added the ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections label Apr 21, 2021
@SNCPlay42
Copy link
Contributor

Reduced:

pub trait Deserialize<'de>: Sized {}
pub trait DeserializeOwned: for<'de> Deserialize<'de> {}

pub trait Extensible {
    type Config;
}

pub trait Installer<C> {
    fn init<B: Extensible<Config = C>>(&mut self) -> ()
    where
        B::Config: DeserializeOwned,
    {
    }
}

@hameerabbasi hameerabbasi removed E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections labels Apr 22, 2021
@apiraino
Copy link
Contributor

apiraino commented Apr 22, 2021

Assigning priority as discussed as part of the Prioritization Working Group and removing I-prioritize.

@rustbot label -I-prioritize +P-high

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Apr 22, 2021
@b-naber
Copy link
Contributor

b-naber commented Apr 23, 2021

I can try to look into this over the weekend.

@jackh726
Copy link
Member

So, a bit of info: We end up trying to prove C: Sized. We end up with two candidates:

ParamCandidate(ConstnessAnd { constness: NotConst, value: Binder(<C as std::marker::Sized>, [Region(BrNamed(DefId(0:6 ~ issue_84398[317d]::DeserializeOwned::'de), 'de))]) })

and

ParamCandidate(ConstnessAnd { constness: NotConst, value: Binder(<C as std::marker::Sized>, []) })

that both evaluate to okay. This is ambiguous and WF checking fails.

@pietroalbini pietroalbini added regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels May 4, 2021
@pietroalbini pietroalbini added this to the 1.53.0 milestone May 4, 2021
@pietroalbini
Copy link
Member

This regression now slipped into beta 1.53.0.

@bors bors closed this as completed in 377d1a9 May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. 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.

9 participants