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

Rust 1.38 regressions weren't fully triaged #65577

Closed
petrochenkov opened this issue Oct 18, 2019 · 6 comments
Closed

Rust 1.38 regressions weren't fully triaged #65577

petrochenkov opened this issue Oct 18, 2019 · 6 comments
Labels
P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@petrochenkov
Copy link
Contributor

petrochenkov commented Oct 18, 2019

A crater run was performed for Rust 1.38 and the digest of regressions is available in #63628 (comment).

The problem is that all regressions starting from "cannot use state because it was mutably borrowed" and below weren't analyzed.
It is too late to fix 1.38 now because it was released several weeks ago, but it's still useful to know what we regressed (there are >100 non-root regressions), some of those errors can be caused by legitimate bugs.

Triage is required mostly from people responsible for the rustc's middle-end (type checking and below).
cc @rust-lang/compiler

@petrochenkov petrochenkov added I-nominated regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 18, 2019
@Centril
Copy link
Contributor

Centril commented Oct 19, 2019

cc @matthewjasper

-- I suspect these are regressions from migrate mode changes?

@matthewjasper
Copy link
Contributor

Changes around migrate mode being ignored for match expressions with guards:

191 cannot use `state` because it was mutably borrowed
 94 cannot assign to `self.input.cached_token` because it is borrowed
 38 cannot borrow `v` as immutable because it is also borrowed as mutable
  4 cannot move out of `_` because it is borrowed
  2 cannot borrow `*value` as immutable because it is also borrowed as mutable
  2 cannot borrow `*self` as mutable more than once at a time
  1 cannot borrow `self.map` as mutable more than once at a time
  1 cannot borrow `*tree` as mutable more than once at a time
  1 cannot borrow `*current` as mutable more than once at a time
  1 assign to part of possibly uninitialized variable: `xgcval`

Type errors caused by failure to resolve a derive macro:

  2 `types::log_level_type::LogLevelType` doesn't implement `std::fmt::Debug`

Require more investigation:

  1 type annotations required: cannot resolve `std::boxed::Box<std::iter::Map<std::collections::hash_map::Iter<'_, &T, std::collections::HashSet<&U>>, [closure@src/weighted_rendezvous.rs:490:40: 493:10]>>: std::iter::Iterator`
  1 type annotations required: cannot resolve `std::boxed::Box<std::iter::Map<std::collections::hash_map::Iter<'_, &T, std::collections::HashSet<&U>>, [closure@src/weighted_rendezvous.rs:483:40: 486:10]>>: std::iter::Iterator`
  1 conflicting implementations of trait `std::convert::From<main::LogarithmicKnob>` for type `main::Knob`:
  1 conflicting implementations of trait `std::convert::From<main::LinearKnob>` for type `main::Knob`:
  1 conflicting implementations of trait `main::KnobControl` for type `main::Knob`:

@pnkfelix
Copy link
Member

triage: P-high for finishing the triage. (Muchas gracias to @matthewjasper for the triage they did up above, reducing it down to only five cases remaining to investigate.)

Leaving nominated for discussion, because I would like to know if we failed to follow through on triaging the regressions because of some process failure, or was the process working as best it could, and we simply ran out of time to complete the triage?

@pnkfelix pnkfelix added the P-high High priority label Oct 24, 2019
@Mark-Simulacrum
Copy link
Member

Mostly process failure -- I had thought that we had already triaged all the regressions given @petrochenkov's comments on the crater issue, and as such did not follow up to file issues that would've brought it to T-compiler's attention. I expect that the rule going forward should be that triage never is just leaving comments, we should always file at least one issue with T-compiler nomination.

@pnkfelix
Copy link
Member

triage: Leaving P-high for the actual triage. Removing nomination; @Mark-Simulacrum 's comment above answered my question about the origin of the failure (and how they hope to address it in the future)

@Centril
Copy link
Contributor

Centril commented Mar 25, 2020

This seems rather moot now that we're in 1.44 nightly, so let's close the issue as no-action-is-likely-to-be-taken.

@Centril Centril closed this as completed Mar 25, 2020
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-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants