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: conflicting implementations between #[fundamental] generic type and FnPtr #124449

Closed
finnbear opened this issue Apr 27, 2024 · 5 comments
Labels
C-bug Category: This is a bug. requires-nightly This issue requires a nightly compiler in some way.

Comments

@finnbear
Copy link
Contributor

finnbear commented Apr 27, 2024

Code

I was changing rust nightly versions while having code like this:

#![feature(fundamental)]

#[derive(PartialEq)]
#[fundamental]
struct Foo<T> {
    bar: T,
}

fn main() {}

I expected to see this happen: It compiles

Instead, this happened:

error[E0119]: conflicting implementations of trait `PartialEq` for type `Foo<_>`
 --> src/main.rs:5:10
  |
5 | #[derive(PartialEq)]
  |          ^^^^^^^^^
  |
  = note: conflicting implementation in crate `core`:
          - impl<F> PartialEq for F
            where F: FnPtr;
  = note: downstream crates may implement trait `std::marker::FnPtr` for type `Foo<_>`
  = note: this error originates in the derive macro `PartialEq` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0119`.
error: could not compile `bisect_fund` (bin "bisect_fund") due to 1 previous error

Versions (cargo bisect-rustc)

# I was upgrading my code between these versions when I noticed the regression
$: cargo bisect-rustc --start=2023-04-25 --end=2024-04-20
...
looking for regression commit between 2023-10-26 and 2023-10-27
...
found 12 bors merge commits in the specified range
  commit[0] 2023-10-25: Auto merge of #117180 - matthiaskrgr:rollup-rxhl6ep, r=matthiaskrgr
  commit[1] 2023-10-25: Auto merge of #117193 - matthiaskrgr:rollup-bygfdcd, r=matthiaskrgr
  commit[2] 2023-10-26: Auto merge of #115872 - ferrocene:pa-remap-cargo-home, r=clubby789
  commit[3] 2023-10-26: Auto merge of #116818 - Nilstrieb:stop-submitting-bug-reports, r=wesleywiser
  commit[4] 2023-10-26: Auto merge of #117115 - zetafunction:linking, r=bjorn3
  commit[5] 2023-10-26: Auto merge of #117148 - dtolnay:sinceversion, r=cjgillot
  commit[6] 2023-10-26: Auto merge of #116983 - Urgau:prepare-bootstrap-for-new-check-cfg, r=Kobzol
  commit[7] 2023-10-26: Auto merge of #112875 - compiler-errors:negative-coherence-rework, r=lcnr
  commit[8] 2023-10-26: Auto merge of #113262 - Nilstrieb:rawr-casting, r=lcnr
  commit[9] 2023-10-26: Auto merge of #117171 - fee1-dead-contrib:deny-explicit-effect-params, r=oli-obk
  commit[10] 2023-10-26: Auto merge of #117228 - matthiaskrgr:rollup-23zzepv, r=matthiaskrgr
  commit[11] 2023-10-26: Auto merge of #116581 - Kobzol:bootstrap-cmd-run, r=onur-ozkan
ERROR: no CI builds available between ab5c841a1f3c09edc5ea07722519627c960aed17 and aa1a71e9e90f6eb3aed8cf79fc80bea304c17ecb within last 167 days

Upon investigating all 12 commits, my best guess is that #112875 introduced this regression, hence mentioning @compiler-errors. My reading of that issue does not lead me to believe this chance was intentional, hence a 'regression', unless the term 'regression' isn't applicable to nightly-to-nightly.

Ideas

Perhaps downstream crates may implement trait std::marker::FnPtr for type Foo<_> should be ignored, since impl FnPtr is disallowed?

Workarounds?

The workaround of impl <T> !FnPtr Foo<T> {} doesn't work since impl FnPtr isn't allowed.

Making Foo concrete avoids the compile error, but my code requires the type parameter.

#![feature(fundamental)]

#[derive(PartialEq)]
#[fundamental]
struct Foo {
    bar: String,
}

Thanks for looking into this!

@rustbot modify labels: +regression-from-nightly-to-nightly -regression-untriaged

@finnbear finnbear added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Apr 27, 2024
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Apr 27, 2024
@compiler-errors
Copy link
Member

It's likely due to 66d7cfd.

This isn't probably worth tracking as a regression since fundamental should probably be marked an internal feature, since the semantics of #[fundamental] (especially its interaction w blankets and rustc_deny_explicit_impl impls) is kinda a detail we shouldn't expose...

@compiler-errors compiler-errors added requires-nightly This issue requires a nightly compiler in some way. and removed regression-untriaged Untriaged performance or correctness regression. labels Apr 27, 2024
@workingjubilee
Copy link
Member

My reading of that issue does not lead me to believe this chance was intentional, hence a 'regression', unless the term 'regression' isn't applicable to nightly-to-nightly.

fwiw nightly-to-nightly is considered a regression usually, you just were indeed using what I am... pretty sure was always intended to be a perma-unstable API?

@finnbear
Copy link
Contributor Author

finnbear commented Apr 28, 2024

Regardless of stability, I'm under the impression that the old behavior is desirable and recoverable (such as by ignoring conflict implementations of FnPtr if impl FnPtr is truly disallowed). However, if there is intent or motivation to keep the new behavior, you're right that it wouldn't violate any stability guarantee.

Maybe someone knows why the same issue doesn't affect Box, which is also #[fundamental], generic, and implements PartialEq. That could be a workaround.

@compiler-errors
Copy link
Member

Maybe someone knows why the same issue doesn't affect Box, which is also #[fundamental], generic, and implements PartialEq. That could be a workaround.

Oh, right; because of negative coherence. We have an implicit impl !FnPtr for most types built-in to the compiler.

@finnbear: You can add #![feature(with_negative_coherence)] to your crate to make this work.

@finnbear
Copy link
Contributor Author

finnbear commented Apr 28, 2024

You can add #![feature(with_negative_coherence)] to your crate to make this work.

Thanks very much, I think this solves my problem (cannot be 100% yet since still dealing with new ICE's). Closing as there is less motivation to restore the old behavior now that a workaround exists.

@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Apr 28, 2024
@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. requires-nightly This issue requires a nightly compiler in some way.
Projects
None yet
Development

No branches or pull requests

6 participants