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

regression: ICE adjusting Expr can't compose Pointer(Unsize) #94511

Closed
Mark-Simulacrum opened this issue Mar 1, 2022 · 14 comments
Closed

regression: ICE adjusting Expr can't compose Pointer(Unsize) #94511

Mark-Simulacrum opened this issue Mar 1, 2022 · 14 comments
Assignees
Labels
E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-low Low 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

@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented Mar 1, 2022

Crater picked up this issue:

stack backtrace:
[INFO] [stdout] error: internal compiler error: compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs:318:26: while adjusting Expr { hir_id: HirId { owner: DefId(0:181 ~ conspect[9ddb]::manager::{impl#0}::match_ns), local_id: 68 }, kind: Box(Expr { hir_id: HirId { owner: DefId(0:181 ~ conspect[9ddb]::manager::{impl#0}::match_ns), local_id: 67 }, kind: Array([Expr { hir_id: HirId { owner: DefId(0:181 ~ conspect[9ddb]::manager::{impl#0}::match_ns), local_id: 66 }, kind: Path(Resolved(None, Path { span: src/manager/mod.rs:40:41: 40:48 (#0), res: Local(HirId { owner: DefId(0:181 ~ conspect[9ddb]::manager::{impl#0}::match_ns), local_id: 4 }), segments: [PathSegment { ident: process#0, hir_id: Some(HirId { owner: DefId(0:181 ~ conspect[9ddb]::manager::{impl#0}::match_ns), local_id: 65 }), res: Some(Local(HirId { owner: DefId(0:181 ~ conspect[9ddb]::manager::{impl#0}::match_ns), local_id: 4 })), args: None, infer_args: true }] })), span: src/manager/mod.rs:40:41: 40:48 (#0) }]), span: /rustc/0a4f984a87c7ba6c74ec3e78442fec955a419e32/library/alloc/src/macros.rs:50:56: 50:65 (#32) }), span: /rustc/0a4f984a87c7ba6c74ec3e78442fec955a419e32/library/alloc/src/macros.rs:50:52: 50:65 (#32) }, can't compose [Pointer(Unsize) -> _] and [Pointer(Unsize) -> _]
@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. labels Mar 1, 2022
@Mark-Simulacrum Mark-Simulacrum added this to the 1.60.0 milestone Mar 1, 2022
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Mar 1, 2022
@oli-obk oli-obk added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-low Low priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Mar 2, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Mar 2, 2022

Should probably become a delay span bug, I don't see this happening unless there are other errors involved

@oli-obk oli-obk added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Mar 2, 2022
@compiler-errors
Copy link
Member

This is (probably) fixed in #94438. As soon as I fix #94516, we should probably backport both of those together, because there is a potential for more ICEs like this. I can look into making this an MVCE.

@compiler-errors
Copy link
Member

Minimization is included as test in #94596

@compiler-errors
Copy link
Member

@rustbot claim

@compiler-errors
Copy link
Member

The fixes for this were beta-backpoorted in #94933, should we close this @Mark-Simulacrum?

@Mark-Simulacrum
Copy link
Member Author

I'm rerunning crater and will close if it doesn't appear again.

@compiler-errors
Copy link
Member

I believe these are fixed in #94636, which was an unrelated bug that may have regressed due to #93118 (or other PRs that were reworking the same method param logic in January).

@Mark-Simulacrum: Want me to do a bisection to find the blame, or to prove that #94636 was in fact the fix? Seems like all of those crates are very broken for other reasons, so not sure if we want to beta-backport the fix either.

@Mark-Simulacrum
Copy link
Member Author

These are just the ones crater picked up, but if there's a simple enough fix that we could backport it seems like we should do so. #94636 doesn't look too bad.

I'm not sure a bisection is the right step -- I'd probably suggest building beta + #94636 backported and see if that's still ICEing on these crates, and if not, we just backport #94636. (If it is, then a bisect is likely the right call).

@compiler-errors
Copy link
Member

Cool, alright, I will just try cherry-picking #94636 then.

@compiler-errors
Copy link
Member

compiler-errors commented Mar 18, 2022

Ah, so #94596 was backported incorrectly I think. An if-statement was left in causing ICEs.

I put up a PR with the change in #95093, though I think it may need to get backport-nominated? idk how backports work, lol.

#94636 fixes another error that looks similar, but I'm not sure if it's worth backporting, and that would need to be nominated I think since it's totally unrelated.

bors added a commit to rust-lang-ci/rust that referenced this issue Mar 22, 2022
…k-Simulacrum

[beta] Remove statement that was forgotten when backporting rust-lang#94596

This `if` statement was introduced in rust-lang#94438, then removed in rust-lang#94596. Both of these PRs were beta-backported in rust-lang#94933, but I think there was a mistake in the order they were applied or this removal was overlooked. I think this fixes the remaining issues referenced in rust-lang#94511 (comment).

Not sure this is the correct way to put something up for beta-backport, but the PR is at least open so it can be referenced and the commit can be cherry-picked. Feel free to close this PR.

r? `@Mark-Simulacrum`
cc: rust-lang#94511
@Mark-Simulacrum
Copy link
Member Author

#95093 was merged into beta, so I think this is likely fixed, and in any case the only known case had other errors (it was a build-fail -> ICE), so we should be OK for 1.60 here.

@pietroalbini
Copy link
Member

Can we close this then?

@compiler-errors
Copy link
Member

I'm just gonna close this then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-low Low 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

No branches or pull requests

5 participants