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

Add core::convert::absurd #124311

Closed
wants to merge 1 commit into from
Closed

Conversation

WaffleLapkin
Copy link
Member

r? @Amanieu

I decided to split this from #122412, as this is solely T-libs-api, and can (should?) be dealt with separately from lang/compiler questions.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 23, 2024
@kpreid
Copy link
Contributor

kpreid commented Apr 23, 2024

@rustbot label -T-libs +T-libs-api

@rustbot rustbot added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 23, 2024
@rust-log-analyzer

This comment has been minimized.

@jhpratt
Copy link
Member

jhpratt commented Apr 24, 2024

LGTM, but not giving the approval as you explicitly requested someone else.

/// let x: u32 = x.unwrap_or_else(absurd);
/// ```
///
/// [`!`]: ../../std/primitive.never.html
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// [`!`]: ../../std/primitive.never.html
/// [`!`]: prim@never

@Amanieu
Copy link
Member

Amanieu commented May 16, 2024

This was discussed in library team meetings and in person at RustNL and we decided not to move forward with this.

@Amanieu Amanieu closed this May 16, 2024
@WaffleLapkin WaffleLapkin deleted the absurd branch May 16, 2024 15:26
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 20, 2024
…rovements, r=compiler-errors

Never type unsafe lint improvements

- Move linting code to a separate method
- Remove mentions of `core::convert::absurd` (rust-lang#124311 was rejected)
- Make the lint into FCW

The last thing is a bit weird though. On one hand it should be `EditionSemanticsChange(2024)`, but on the other hand it shouldn't, because we also plan to break it on all editions some time later. _Also_, it's weird that we don't have `FutureReleaseSemanticsChangeReportInDeps`, IMO "this might cause UB in a future release" is important enough to be reported in deps...

IMO we ought to have three enums instead of [`FutureIncompatibilityReason`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint_defs/enum.FutureIncompatibilityReason.html#):

```rust
enum IncompatibilityWhen {
     FutureRelease,
     Edition(Edition),
}

enum IncompatibilyWhat {
    Error,
    SemanticChange,
}

enum IncompatibilityReportInDeps {
    No,
    Yes,
}
```

Tracking:
- rust-lang#123748
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 20, 2024
Rollup merge of rust-lang#125282 - WaffleLapkin:never-type-unsafe-improvements, r=compiler-errors

Never type unsafe lint improvements

- Move linting code to a separate method
- Remove mentions of `core::convert::absurd` (rust-lang#124311 was rejected)
- Make the lint into FCW

The last thing is a bit weird though. On one hand it should be `EditionSemanticsChange(2024)`, but on the other hand it shouldn't, because we also plan to break it on all editions some time later. _Also_, it's weird that we don't have `FutureReleaseSemanticsChangeReportInDeps`, IMO "this might cause UB in a future release" is important enough to be reported in deps...

IMO we ought to have three enums instead of [`FutureIncompatibilityReason`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint_defs/enum.FutureIncompatibilityReason.html#):

```rust
enum IncompatibilityWhen {
     FutureRelease,
     Edition(Edition),
}

enum IncompatibilyWhat {
    Error,
    SemanticChange,
}

enum IncompatibilityReportInDeps {
    No,
    Yes,
}
```

Tracking:
- rust-lang#123748
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants