Don't check for late-bound vars, check for escaping bound vars #11760
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @Jarcho (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
the
S-waiting-on-review
|
There was a test here #11230 (comment) can you just throw that into |
rust-cloud-vms
bot
force-pushed
the
escaping
branch
from
3f9fb6d
to
ff386e2
Compare
|
I actually just removed the |
|
The |
|
i confirmed that this fixes #11230 when applied on top of rust-lang/rust#117595 :) |
|
Ping @compiler-errors. Can you re-add the debug @rustbot author |
rustbot
added
S-waiting-on-author
compiler-errors
force-pushed
the
escaping
branch
from
ff386e2
to
1539eb8
Compare
|
@rustbot ready |
rustbot
added
S-waiting-on-review
|
Thank you. @bors r+ I assume an escaped variable is one that is missing it's corresponding binder, correct? |
bors
added a commit
that referenced
this pull request
Nov 12, 2023
Don't check for late-bound vars, check for escaping bound vars Fixes an assertion that didn't make sense. Many valid and well-formed types *have* late-bound vars (e.g. `for<'a> fn(&'a ())`), they just must not have *escaping* late-bound vars in order to be normalized correctly. Addresses #11230, cc `@jyn514` and `@matthiaskrgr`
|
💔 Test failed - checks-action_test |
|
lol, I have added the missing |
|
@bors r=Jarcho |
|
💡 This pull request was already approved, no need to approve it again. |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
matthiaskrgr
added
the
beta-nominated
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
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 pull request
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 pull request
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 pull request
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 pull request
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 pull request
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).
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Dec 21, 2023
…troalbini [beta] Clippy beta backport PR towards stable, as beta was branched a day early and I missed the notification. - rust-lang/rust-clippy#11538 - rust-lang/rust-clippy#11756 - rust-lang/rust-clippy#11760 - rust-lang/rust-clippy#11953 r? `@pietroalbini`
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Dec 21, 2023
…troalbini [beta] Clippy beta backport PR towards stable, as beta was branched a day early and I missed the notification. - rust-lang/rust-clippy#11538 - rust-lang/rust-clippy#11756 - rust-lang/rust-clippy#11760 - rust-lang/rust-clippy#11953 r? `@pietroalbini`
flip1995
added
beta-accepted
xFrednet
removed
the
beta-accepted
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes an assertion that didn't make sense. Many valid and well-formed types have late-bound vars (e.g.
for<'a> fn(&'a ())), they just must not have escaping late-bound vars in order to be normalized correctly.Addresses #11230, cc @jyn514 and @matthiaskrgr
changelog: don't check for late-bound vars, check for escaping bound vars. Addresses #11230