Skip to content

NLL Regressions in 1.40 #66517

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

Closed
Mark-Simulacrum opened this issue Nov 18, 2019 · 10 comments
Closed

NLL Regressions in 1.40 #66517

Mark-Simulacrum opened this issue Nov 18, 2019 · 10 comments
Assignees
Labels
A-NLL Area: Non-lexical lifetimes (NLL) 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. T-release Relevant to the release subteam, which will review and decide on the PR/issue.

Comments

@Mark-Simulacrum
Copy link
Member

root: capnp - 4 (1 gh, 3 crates.io) detected crates which regressed due to this:

root: epub - 4 (2 gh, 2 crates.io) detected crates which regressed due to this

root: gfx-hal - 4 (2 gh, 2 crates.io) detected crates which regressed due to this

root: gltf - 10 (6 gh, 4 crates.io) detected crates which regressed due to this:

root: gstreamer - 3 (3 gh, 0 crates.io) detected crates which regressed due to this

root: liquid-value - 6 (4 gh, 2 crates.io) detected crates which regressed due to this

@Mark-Simulacrum Mark-Simulacrum added 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. T-release Relevant to the release subteam, which will review and decide on the PR/issue. labels Nov 18, 2019
@Centril
Copy link
Contributor

Centril commented Nov 19, 2019

Is there anything actionable here?

@jonas-schievink jonas-schievink added the A-NLL Area: Non-lexical lifetimes (NLL) label Nov 19, 2019
@Mark-Simulacrum
Copy link
Member Author

I am hopeful that we can patch crates on crates.io at least, ideally we'd go out and fix all of the above (i.e., PRs, or better yet releases where applicable) in time for the release.

@pnkfelix
Copy link
Member

triage: P-high

@pnkfelix pnkfelix added the P-high High priority label Nov 21, 2019
@pnkfelix
Copy link
Member

nominating for discussion at triage meeting, to see if we can get a subset of developers to divvy up the work here.

@lqd
Copy link
Member

lqd commented Nov 21, 2019

I took a look at

  1. chiisai-0.1.6: the repository has been archived
  2. the capnp root:
  • capnp-gj-0.2.1, depends on capnp-0.7.5: the repository was marked deprecated in 2017
  • leaf-0.2.1, depends on capnp-0.6.2: the project has been abandoned in 2016
  • rotor-capnp-0.1.0, depends on capnp-0.6.2: the repository looks inactive since 2016
  • dmichiels.temp_rustfbp, depends on capnp-0.7.5: the repository looks like a test, inactive since 2016
  1. the gfx-hal root:
  • bvh_anim-0.4.0, depends on gfx-hal-0.1.0 via wgpu-0.2.3: master has since been fixed when it removed the dependency, but updating to wgpu-0.3.0 also makes it build (probably because it's not actually used as a dependency)
  • imagine-0.0.1, depends on gfx-hal-0.1.0: a temporary "squatting" release, which builds with e.g. gfx-hal-0.2.0
  • lcnr/wgpu_err, depends on gfx-hal-0.1.0 via wgpu-0.2.3: the single commit repository mentions it's only a test -- but is otherwise in the same boat as the next one
  • vitvakatu/cubes, depends on gfx-hal-0.1.0 via wgpu-0.2.2: seems like a port of a demo, last commit 8 months ago -- note that wgpu-0.3.0's gfx-hal version builds, but is itself incompatible with wgpu-0.2, so maybe we could somehow bump gfx-hal in another wgpu-0.2 release ?

Here's a hackmd for easier coordination, at the very least to filter out the inactive crates.

@pnkfelix
Copy link
Member

discussed this in last week's T-compiler meeting.

Removing nomination and self-assigning to push through remainder of review.

@pnkfelix pnkfelix self-assigned this Nov 28, 2019
@Mark-Simulacrum
Copy link
Member Author

I think I actually finished most of the bullets in the hackmd, mostly concluding that there's no need to take any particular action. I believe there's a few unchecked bullets left that probably warrant some deeper investigation (perhaps a PR).

@lqd
Copy link
Member

lqd commented Nov 29, 2019

As another update, there was also coordination in this zulip thread.

We've gone over the list, sent a few PRs where we could (many thanks to @djc for helping with this as well), and it seems like we're in an OK place.

@lqd
Copy link
Member

lqd commented Nov 29, 2019

also, many thanks to @P-E-Meunier for publishing the semver-compatible updates 0.19.6 and 0.20.8 to thrussh 🎉

@Mark-Simulacrum
Copy link
Member Author

I believe we are now ready to close this issue, deeming the rest of the regressions as either won't fix (due to the crate/repository in question being deprecated or otherwise unmaintained) or fixable via cargo update (and we don't have the resources to send hundreds of PRs doing so).

@Centril Centril closed this as completed Nov 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-NLL Area: Non-lexical lifetimes (NLL) 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. T-release Relevant to the release subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants