Skip to content

remove unreachable error code E0476#106840

Closed
Ezrashaw wants to merge 1 commit into
rust-lang:masterfrom
Ezrashaw:remove-e0476
Closed

remove unreachable error code E0476#106840
Ezrashaw wants to merge 1 commit into
rust-lang:masterfrom
Ezrashaw:remove-e0476

Conversation

@Ezrashaw
Copy link
Copy Markdown
Contributor

Remove E0476 unreachable/undocumented/untested error code.

r? @GuillaumeGomez (I don't think you've noticed my other PRs for #61137 so I'll do more than just ping)

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 14, 2023
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Jan 14, 2023

Some changes occurred in diagnostic error codes

cc @GuillaumeGomez

@GuillaumeGomez
Copy link
Copy Markdown
Member

Looks good to me but since it's changing compiler's code, we'll need someone from their team.

r? @estebank

};
let (source, target, trait_def_id, kind) = match (source.kind(), target.kind()) {
(&ty::Ref(r_a, ty_a, mutbl_a), &ty::Ref(r_b, ty_b, mutbl_b)) => {
infcx.sub_regions(infer::RelateObjectBound(span), r_b, r_a);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this line removed? This has side-effects.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could give it a different subregion origin, but this is required for soundness 😓

Copy link
Copy Markdown
Contributor

@compiler-errors compiler-errors Jan 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code reproduces this error:

#![feature(coerce_unsized)]
#![feature(unsize)]

use core::marker::Unsize;
use core::ops::CoerceUnsized;

struct Wrapper<T>(T);

impl<'a, 'b, T, S> CoerceUnsized<&'a Wrapper<T>> for &'b Wrapper<S> where S: Unsize<T> {}

Although it has a coherence error as well. Presumably if you made https://doc.rust-lang.org/src/core/ops/unsize.rs.html#55 this implementation unsound by removing 'b: 'a, then it would also trigger there.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, my mistake, I assumed that this error code was no longer used. I'll close this PR and document/test it instead.

Copy link
Copy Markdown
Contributor

@compiler-errors compiler-errors Jan 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's alright -- in general, I would never remove side-effect-ful logic just because you think it's dead code. Sometimes people just forget to add a UI test for an error code.

Replace it with a delay_span_bug or bug!() if necessary, but unless you can statically prove that something is unreachable, you're very likely to introduce unsoundness or other subtle bugs into the compiler.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I was kind of naive. I assumed that untested error codes (basically all of them are like this) were from old Rust versions and had never been formally removed.

@Ezrashaw Ezrashaw closed this Jan 14, 2023
@Ezrashaw Ezrashaw deleted the remove-e0476 branch February 28, 2023 07:40
@Ezrashaw Ezrashaw restored the remove-e0476 branch May 3, 2024 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants