Skip to content

Define coherence restrictions in the manual impl section. #9743

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

Closed
nejucomo opened this issue Oct 6, 2013 · 5 comments
Closed

Define coherence restrictions in the manual impl section. #9743

nejucomo opened this issue Oct 6, 2013 · 5 comments

Comments

@nejucomo
Copy link

nejucomo commented Oct 6, 2013

I'm familiar with coherence restrictions on trait impls only from IRC conversations ages ago. It came up in IRC again, and I briefly skimmed the manual sections on impls and crates to see if this restriction was spelled out.

I may have missed it. Even in that case, I feel it would be most useful in the manual section describing impls: 6.1.9 Implementations (v0.8 manual)

pcwalton has a good blog post providing background: Coherence, Modularity, and Extensibility for Typeclasses

There was some confusion on IRC as to whether coherence restrictions are per-module or per-crate.

Some questions I'd like to see answered in the manual:

  • Are trait impls "always public" even when buried in non-public modules?
  • Can a crate define a trait in one module, A, and an impl in another module B coherently?
@huonw huonw added the A-docs label Feb 3, 2014
@huonw
Copy link
Member

huonw commented Feb 3, 2014

Triage: coherence is per-crate, since the compiler sees the whole crate at once (and so can reason about that).

Answers (if someone wishes to take this on):

Are trait impls "always public" even when buried in non-public modules?

Yes, impls are scoped with the trait, so if the trait is public, so are all impls no matter where the impls are placed.

Can a crate define a trait in one module, A, and an impl in another module B coherently?

Yes, that's fine, since coherence is per-crate.

@pnkfelix
Copy link
Member

pnkfelix commented Feb 3, 2014

This seems like a sub-bug of #11166 to me...

@huonw
Copy link
Member

huonw commented Feb 3, 2014

@pnkfelix pointed out that I'd accidentally said "coherence is per-module" originally; fixed now.

@reem
Copy link
Contributor

reem commented Dec 15, 2014

Triage: @pnkfelix is correct that this is part of #11166, so should be closed or #11166 should be converted to a meta-bug.

@steveklabnik
Copy link
Member

I'm just going to close this in favor of that one.

flip1995 pushed a commit to flip1995/rust that referenced this issue Nov 21, 2022
…=Alexendoo

Improve `needless_lifetimes`

This PR makes the following improvements to `needless_lifetimes`.

* It fixes the following false negative, where `foo` is flagged but `bar` is not:
  ```rust
    fn foo<'a>(x: &'a u8, y: &'_ u8) {}

    fn bar<'a>(x: &'a u8, y: &'_ u8, z: &'_ u8) {}
  ```
* It flags more cases, generally. Previously, `needless_borrow` required *all* lifetimes to be used only once. With the changes, individual lifetimes are flagged for being used only once, even if not all lifetimes are.
* Finally, it tries to produce more clear error messages.

changelog: fix `needless_lifetimes` false negative involving functions with multiple unnamed lifetimes
changelog: in `needless_lifetimes`, flag individual lifetimes used only once, rather than require all lifetimes to be used only once
changelog: in `needless_lifetimes`, emit "replace with `'_`" warnings only when applicable, and point to a generic argument
flip1995 pushed a commit to flip1995/rust that referenced this issue Dec 1, 2022
…ndoo

Use `walk_generic_arg`

This is a tiny followup to to rust-lang#9743, now that rust-lang#103692 has landed.

r? `@Alexendoo`

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants