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

Future compatibility lints are worded too strongly #120887

Open
QuineDot opened this issue Feb 10, 2024 · 2 comments
Open

Future compatibility lints are worded too strongly #120887

QuineDot opened this issue Feb 10, 2024 · 2 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@QuineDot
Copy link

Code

trait Trait {}
impl Trait for fn(&'static str) {}
impl Trait for fn(&str) {}

Current output

warning: conflicting implementations of trait `Trait` for type `fn(&'static str)`
 --> src/lib.rs:3:1
  |
2 | impl Trait for fn(&'static str) {}
  | ------------------------------- first implementation here
3 | impl Trait for fn(&str) {}
  | ^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `fn(&'static str)`
  |
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue #56105 <https://github.com/rust-lang/rust/issues/56105>
  = note: this behavior recently changed as a result of a bug fix; see rust-lang/rust#56105 for details
  = note: `#[warn(coherence_leak_check)]` on by default

Desired output

warning: conflicting implementations of trait `Trait` for type `fn(&'static str)`
 --> src/lib.rs:3:1
  |
2 | impl Trait for fn(&'static str) {}
  | ------------------------------- first implementation here
3 | impl Trait for fn(&str) {}
  | ^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `fn(&'static str)`
  |
  = warning: this was previously accepted by the compiler but is problematic; it may become a hard error in a future release!
  = note: for more information, see issue #56105 <https://github.com/rust-lang/rust/issues/56105>
  = note: this behavior recently changed as a result of a bug fix; see rust-lang/rust#56105 for details
  = note: `#[warn(coherence_leak_check)]` on by default

Rationale and extra context

Future compatibility lints assert that the linted code will become a hard error in the future. However, not all future compatibility lints become hard errors. The two-phase borrow compatibility lint is one example that never became a hard error. As far as I know, it is still the plan that the provided example remains supported as well. And I've seen other cases as well.

The assertion that the code will definitely become a hard error can be problematic if people rely on it for new compile/language development, or if they rely on it when reasoning about making their unsafe code sound. It's also just plain annoying to find out the compiler is telling you and everyone else something which isn't true.

The specific future compatibility lint from my code example is just an example; all such lints share the "it will become a hard error" verbiage.

Other cases

No response

Rust Version

Stable channel

Build using the Stable version: 1.76.0
Beta channel

Build using the Beta version: 1.77.0-beta.2

(2024-02-09 f2048098a1ca42a520e7)
Nightly channel

Build using the Nightly version: 1.78.0-nightly

(2024-02-09 d44e3b95cb9d410d89cb)

Anything else?

No response

@QuineDot QuineDot added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 10, 2024
@fmease
Copy link
Member

fmease commented Feb 10, 2024

#120716 will change the wording for the example you gave.

@QuineDot
Copy link
Author

I still feel a general change is appropriate as this isn't a one-time occurance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints 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

2 participants