-
-
Notifications
You must be signed in to change notification settings - Fork 14.9k
Defaulted unit types no longer error out (regression?) #51125
Copy link
Copy link
Open
Labels
A-inferenceArea: Type inferenceArea: Type inferenceC-bugCategory: This is a bug.Category: This is a bug.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.regression-from-stable-to-stablePerformance or correctness regression from one stable version to another.Performance or correctness regression from one stable version to another.
Metadata
Metadata
Assignees
Labels
A-inferenceArea: Type inferenceArea: Type inferenceC-bugCategory: This is a bug.Category: This is a bug.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.regression-from-stable-to-stablePerformance or correctness regression from one stable version to another.Performance or correctness regression from one stable version to another.
Type
Fields
Give feedbackNo fields configured for issues without a type.
This currently compiles (on stable and nightly). Till 1.25, it would trigger a lint because it has inference default to
()instead of throwing an error.(playpen)
That lint indicates that it would become a hard error in the future, but it's not erroring. It seems like a bunch of this was changed when we stabilized
!.That issue says
Though this doesn't really make sense, this looks like a safe way to produce nevers, which should, in short, never happen. It seems like this is related to #40801 -- but that was closed as it seems to be a more drastic change.
Also, if you print
val, it's clear that the compiler thought it was a unit type. This seems like one of those cases where attempting to observe the situation changes it.We should have a hard error here, this looks like a footgun, and as @SimonSapin mentioned has broken some unsafe code already. We had an upgrade lint for the hard error and we'll need to reintroduce it for a cycle or two since we've had a release without it. AFAICT this is a less drastic change than #40801, and we seem to have intended for this to be a hard error before so it probably is minor enough that it's fine.
cc @nikomatsakis @eddyb
h/t @spacekookie for finding this