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

Nightly 65e297d1e 2023-11-03 fails to infer auto traits #117602

Closed
adamreichold opened this issue Nov 5, 2023 · 5 comments · Fixed by #117610
Closed

Nightly 65e297d1e 2023-11-03 fails to infer auto traits #117602

adamreichold opened this issue Nov 5, 2023 · 5 comments · Fixed by #117610
Labels
A-inference Area: Type inference C-bug Category: This is a bug. F-generic_associated_types `#![feature(generic_associated_types)]` a.k.a. GATs T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@adamreichold
Copy link
Contributor

adamreichold commented Nov 5, 2023

The type aho_corasick::AhoCorack is not inferred to be Send + Sync by the above mentioned nightly release, c.f. https://github.com/quickwit-oss/tantivy/actions/runs/6759263517/job/18371750242?pr=2239 Using the stable, the code compiles with out issue.

EDIT: It seems to be related to GAT, i.e. a more minimal reproducer is:

trait SomethingSomething: Send {}

struct Wrapper<T> {
    something: Box<dyn SomethingSomething>,
    other: T,
}

trait HasSend {
    type IsSend<T: Send>: Send;
}

impl<T> HasSend for Wrapper<T> {
    type IsSend<S: Send> = Wrapper<S>;
}

which similarly fails on nightly and builds on stable.

More details
> rustc --crate-type=lib nightly-auto-trait.rs 
error[E0277]: `(dyn SomethingSomething + 'static)` cannot be sent between threads safely
   --> nightly-auto-trait.rs:13:28
    |
13  |     type IsSend<S: Send> = Wrapper<S>;
    |                            ^^^^^^^^^^ `(dyn SomethingSomething + 'static)` cannot be sent between threads safely
    |
    = help: the trait `Send` is not implemented for `(dyn SomethingSomething + 'static)`
    = note: required for `Unique<(dyn SomethingSomething + 'static)>` to implement `Send`
note: required because it appears within the type `Box<dyn SomethingSomething>`
   --> /home/adam/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:195:12
    |
195 | pub struct Box<
    |            ^^^
note: required because it appears within the type `Wrapper<S>`
   --> nightly-auto-trait.rs:3:8
    |
3   | struct Wrapper<T> {
    |        ^^^^^^^
note: required by a bound in `HasSend::IsSend`
   --> nightly-auto-trait.rs:9:27
    |
9   |     type IsSend<T: Send>: Send;
    |                           ^^^^ required by this bound in `HasSend::IsSend`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.

on the above-mentioned nightly version whereas stable is happy

> rustc +stable --crate-type=lib nightly-auto-trait.rs 
warning: fields `something` and `other` are never read
 --> nightly-auto-trait.rs:4:5
  |
3 | struct Wrapper<T> {
  |        ------- fields in this struct
4 |     something: Box<dyn SomethingSomething>,
  |     ^^^^^^^^^
5 |     other: T,
  |     ^^^^^
  |
  = note: `#[warn(dead_code)]` on by default

warning: 1 warning emitted

Simply asserting that Wrapper is Send + Sync via e.g. fn is_send_sync<T: Send + Sync>(_: T) {} works on stable and nightly.

@adamreichold adamreichold added the C-bug Category: This is a bug. label Nov 5, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 5, 2023
@workingjubilee workingjubilee added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-inference Area: Type inference F-generic_associated_types `#![feature(generic_associated_types)]` a.k.a. GATs labels Nov 5, 2023
@lqd
Copy link
Member

lqd commented Nov 5, 2023

The reproducer wouldn't compile on stable though?

@adamreichold
Copy link
Contributor Author

adamreichold commented Nov 5, 2023

The reproducer wouldn't compile on stable though?

Sorry, I must have messed something up when trying to whittle it down as the reproducer I started with definitely did compile on stable. Will try to trace my steps backwards.

@adamreichold
Copy link
Contributor Author

adamreichold commented Nov 5, 2023

And indeed using

struct Wrapper<T> {
    something: Box<dyn SomethingSomething>,
    other: T,
}

instead of

struct Wrapper<T> {
    something: &'static dyn SomethingSomething,
    other: T,
}

makes it compile on stable. (I wanted to avoid Box to avoid the standard library but indeed Box is closer to the original code in question.)

I updated the reproducer in the OP accordingly.

@lqd
Copy link
Member

lqd commented Nov 5, 2023

Great, thank you.

So I believe this to be a different manifestation of #117598 from #117131 cc @compiler-errors -- and therefore also to be fixed by #117542

@ivmarkov
Copy link
Contributor

ivmarkov commented Nov 5, 2023

Stumbled on this separately in my own code. The simplest reproduction I was able to come up with is

pub trait Foo {
    type Gat<F>: Send;
}

pub struct Bar;

impl Foo for Bar {
    type Gat<F> = Box<dyn FnOnce() + Send + 'static>;
}

Removing F makes it compile on nightly as well. Putting a Send bound on F does not help.

@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 5, 2023
@bors bors closed this as completed in 114f1f6 Nov 7, 2023
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 10, 2023
Build Fuchsia in CI

This is very much in a draft state but I wanted to put it up now to get early feedback. It would also be nice if we could test it on actual CI builders.

Fittingly, it is failing right now due to discovering an ICE in clippy, probably more fallout from recent type system changes. Other recent regressions this would have caught include

- rust-lang#117455 and rust-lang#117493
- rust-lang#117602

This ends up not sharing very much at all with cargotest. Fuchsia has its own tool to manage checkouts and its own build system. What it requires is a fully "install"ed toolchain with a host and fuchsia target. We share logic from the dist-various-2 builder to build the fuchsia target.

Right now this runs clippy and skips linking a bunch of targets, since most issues we catch are in the frontend. In theory we could probably get the build CPU time down quite a bit with this approach, but right now some linked targets are creeping into the dependencies anyway and we don't have a good way of preventing that yet.

The approach is basically to get a checkout at a pinned commit and then run a [script](https://fuchsia-review.git.corp.google.com/c/fuchsia/+/943833/6/scripts/rust/build_fuchsia_from_rust_ci.sh) at a predetermined location. I would like to update that pin every few weeks. For now we cherry-pick some build changes but that's temporary. Partial checkouts are used to minimize clone time, but we don't filter out prebuilt packages.

r? `@Mark-Simulacrum`

Based on discussion in [this Zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/Putting.20Fuchsia.20in.20crater).
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 11, 2023
Build Fuchsia in CI

This is very much in a draft state but I wanted to put it up now to get early feedback. It would also be nice if we could test it on actual CI builders.

Fittingly, it is failing right now due to discovering an ICE in clippy, probably more fallout from recent type system changes. Other recent regressions this would have caught include

- rust-lang#117455 and rust-lang#117493
- rust-lang#117602

This ends up not sharing very much at all with cargotest. Fuchsia has its own tool to manage checkouts and its own build system. What it requires is a fully "install"ed toolchain with a host and fuchsia target. We share logic from the dist-various-2 builder to build the fuchsia target.

Right now this runs clippy and skips linking a bunch of targets, since most issues we catch are in the frontend. In theory we could probably get the build CPU time down quite a bit with this approach, but right now some linked targets are creeping into the dependencies anyway and we don't have a good way of preventing that yet.

The approach is basically to get a checkout at a pinned commit and then run a [script](https://fuchsia-review.git.corp.google.com/c/fuchsia/+/943833/6/scripts/rust/build_fuchsia_from_rust_ci.sh) at a predetermined location. I would like to update that pin every few weeks. For now we cherry-pick some build changes but that's temporary. Partial checkouts are used to minimize clone time, but we don't filter out prebuilt packages.

r? `@Mark-Simulacrum`

Based on discussion in [this Zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/Putting.20Fuchsia.20in.20crater).
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 17, 2023
Build Fuchsia in CI

This is very much in a draft state but I wanted to put it up now to get early feedback. It would also be nice if we could test it on actual CI builders.

Fittingly, it is failing right now due to discovering an ICE in clippy (looks like fixed in rust-lang/rust-clippy#11760), probably more fallout from recent type system changes. Other recent regressions this would have caught include

- rust-lang#117455 and rust-lang#117493
- rust-lang#117602

This ends up not sharing very much at all with cargotest. Fuchsia has its own tool to manage checkouts and its own build system. What it requires is a fully "install"ed toolchain with a host and fuchsia target. We share logic from the dist-various-2 builder to build the fuchsia target.

Right now this runs clippy and skips linking a bunch of targets, since most issues we catch are in the frontend. In theory we could probably get the build CPU time down quite a bit with this approach, but right now some linked targets are creeping into the dependencies anyway and we don't have a good way of preventing that yet.

The approach is basically to get a checkout at a pinned commit and then run a [script](https://fuchsia-review.git.corp.google.com/c/fuchsia/+/943833/6/scripts/rust/build_fuchsia_from_rust_ci.sh) at a predetermined location. I would like to update that pin every few weeks. For now we cherry-pick some build changes but that's temporary. Partial checkouts are used to minimize clone time, but we don't filter out prebuilt packages.

r? `@Mark-Simulacrum`

Based on discussion in [this Zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/Putting.20Fuchsia.20in.20crater).
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 17, 2023
Build Fuchsia in CI

This is very much in a draft state but I wanted to put it up now to get early feedback. It would also be nice if we could test it on actual CI builders.

Fittingly, it is failing right now due to discovering an ICE in clippy (looks like fixed in rust-lang/rust-clippy#11760), probably more fallout from recent type system changes. Other recent regressions this would have caught include

- rust-lang#117455 and rust-lang#117493
- rust-lang#117602

This ends up not sharing very much at all with cargotest. Fuchsia has its own tool to manage checkouts and its own build system. What it requires is a fully "install"ed toolchain with a host and fuchsia target. We share logic from the dist-various-2 builder to build the fuchsia target.

Right now this runs clippy and skips linking a bunch of targets, since most issues we catch are in the frontend. In theory we could probably get the build CPU time down quite a bit with this approach, but right now some linked targets are creeping into the dependencies anyway and we don't have a good way of preventing that yet.

The approach is basically to get a checkout at a pinned commit and then run a [script](https://fuchsia-review.git.corp.google.com/c/fuchsia/+/943833/6/scripts/rust/build_fuchsia_from_rust_ci.sh) at a predetermined location. I would like to update that pin every few weeks. For now we cherry-pick some build changes but that's temporary. Partial checkouts are used to minimize clone time, but we don't filter out prebuilt packages.

r? `@Mark-Simulacrum`

Based on discussion in [this Zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/Putting.20Fuchsia.20in.20crater).
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 17, 2023
Build Fuchsia in CI

This is very much in a draft state but I wanted to put it up now to get early feedback. It would also be nice if we could test it on actual CI builders.

Fittingly, it is failing right now due to discovering an ICE in clippy (looks like fixed in rust-lang/rust-clippy#11760), probably more fallout from recent type system changes. Other recent regressions this would have caught include

- rust-lang#117455 and rust-lang#117493
- rust-lang#117602

This ends up not sharing very much at all with cargotest. Fuchsia has its own tool to manage checkouts and its own build system. What it requires is a fully "install"ed toolchain with a host and fuchsia target. We share logic from the dist-various-2 builder to build the fuchsia target.

Right now this runs clippy and skips linking a bunch of targets, since most issues we catch are in the frontend. In theory we could probably get the build CPU time down quite a bit with this approach, but right now some linked targets are creeping into the dependencies anyway and we don't have a good way of preventing that yet.

The approach is basically to get a checkout at a pinned commit and then run a [script](https://fuchsia-review.git.corp.google.com/c/fuchsia/+/943833/6/scripts/rust/build_fuchsia_from_rust_ci.sh) at a predetermined location. I would like to update that pin every few weeks. For now we cherry-pick some build changes but that's temporary. Partial checkouts are used to minimize clone time, but we don't filter out prebuilt packages.

r? `@Mark-Simulacrum`

Based on discussion in [this Zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/Putting.20Fuchsia.20in.20crater).
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 27, 2023
Build Fuchsia in CI

This is very much in a draft state but I wanted to put it up now to get early feedback. It would also be nice if we could test it on actual CI builders.

Fittingly, it is failing right now due to discovering an ICE in clippy (looks like fixed in rust-lang/rust-clippy#11760), probably more fallout from recent type system changes. Other recent regressions this would have caught include

- rust-lang#117455 and rust-lang#117493
- rust-lang#117602

This ends up not sharing very much at all with cargotest. Fuchsia has its own tool to manage checkouts and its own build system. What it requires is a fully "install"ed toolchain with a host and fuchsia target. We share logic from the dist-various-2 builder to build the fuchsia target.

Right now this runs clippy and skips linking a bunch of targets, since most issues we catch are in the frontend. In theory we could probably get the build CPU time down quite a bit with this approach, but right now some linked targets are creeping into the dependencies anyway and we don't have a good way of preventing that yet.

The approach is basically to get a checkout at a pinned commit and then run a [script](https://fuchsia-review.git.corp.google.com/c/fuchsia/+/943833/6/scripts/rust/build_fuchsia_from_rust_ci.sh) at a predetermined location. I would like to update that pin every few weeks. For now we cherry-pick some build changes but that's temporary. Partial checkouts are used to minimize clone time, but we don't filter out prebuilt packages.

r? `@Mark-Simulacrum`

Based on discussion in [this Zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/Putting.20Fuchsia.20in.20crater).
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 27, 2023
Build Fuchsia in CI

This is very much in a draft state but I wanted to put it up now to get early feedback. It would also be nice if we could test it on actual CI builders.

Fittingly, it is failing right now due to discovering an ICE in clippy (looks like fixed in rust-lang/rust-clippy#11760), probably more fallout from recent type system changes. Other recent regressions this would have caught include

- rust-lang#117455 and rust-lang#117493
- rust-lang#117602

This ends up not sharing very much at all with cargotest. Fuchsia has its own tool to manage checkouts and its own build system. What it requires is a fully "install"ed toolchain with a host and fuchsia target. We share logic from the dist-various-2 builder to build the fuchsia target.

Right now this runs clippy and skips linking a bunch of targets, since most issues we catch are in the frontend. In theory we could probably get the build CPU time down quite a bit with this approach, but right now some linked targets are creeping into the dependencies anyway and we don't have a good way of preventing that yet.

The approach is basically to get a checkout at a pinned commit and then run a [script](https://fuchsia-review.git.corp.google.com/c/fuchsia/+/943833/6/scripts/rust/build_fuchsia_from_rust_ci.sh) at a predetermined location. I would like to update that pin every few weeks. For now we cherry-pick some build changes but that's temporary. Partial checkouts are used to minimize clone time, but we don't filter out prebuilt packages.

r? `@Mark-Simulacrum`

Based on discussion in [this Zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/Putting.20Fuchsia.20in.20crater).
bors added a commit to rust-lang-ci/rust that referenced this issue Dec 9, 2023
Build Fuchsia in CI

Fittingly, when I first put this up it was failing due to discovering an ICE in clippy (looks like fixed in rust-lang/rust-clippy#11760), probably more fallout from recent type system changes. Other recent regressions this would have caught include

- rust-lang#117455 and rust-lang#117493
- rust-lang#117602

Originally we discussed basing this on cargotest, but they ended up not sharing anything. Fuchsia has its own tool to manage checkouts and its own build system. What it requires is a fully "install"ed toolchain with a host and fuchsia target. We share logic from the dist-various-2 builder to build the fuchsia target.

Right now this runs clippy and skips linking a bunch of targets, since most issues we catch are in the frontend. In theory we could probably get the build CPU time down quite a bit with this approach, but right now some linked targets are creeping into the dependencies anyway and we don't have a good way of preventing that yet.

The approach is basically to get a checkout at a pinned commit and then run a [script](https://fuchsia-review.git.corp.google.com/c/fuchsia/+/943833/6/scripts/rust/build_fuchsia_from_rust_ci.sh) at a predetermined location. I would like to update that pin every few weeks. Partial checkouts are used to minimize clone time, but we don't filter out prebuilt packages.

r? `@Mark-Simulacrum`

Based on discussion in [this Zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/Putting.20Fuchsia.20in.20crater).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-inference Area: Type inference C-bug Category: This is a bug. F-generic_associated_types `#![feature(generic_associated_types)]` a.k.a. GATs 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.

6 participants