Semantic checks of impl restrictions

[CoCo-Japan-pan]

View all comments

This PR implements semantic checks for impl restrictions proposed in the Restrictions RFC (Tracking Issue #105077), and linked to a GSOC idea/proposal.

It lowers the resolved paths of impl restrictions from the AST to HIR and into TraitDef, and integrates the checks into the coherence phase by extending check_impl. As parsing (#152943) and path resolution (#153556) have already been implemented, this PR provides a working implementation of impl restrictions.

r? @Urgau
cc @jhpratt

[rustbot]

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

[Urgau]

Looks promising. Left some preliminary comments.

View changes since this review

[Urgau]

Looks good to me. I will let @jhpratt do the final review.

View changes since this review

[jhpratt]

View changes since the review

As a separate crate, shouldn't this be simply outside `external`? A downstream user shouldn't need to know the specific module that it can't be implemented outside of, as it's not in their control. Naming the external crate should be sufficient — anywhere they try to implement it in their crate will fail.

At least, I'm fairly certain that's why I added this test in my original PR; I wanted to be sure the conditional was working properly.

[Urgau]

I've asked for the condition to be changed in #154661 (comment) as it was refering to the wrong crate name (external_impl_restriction instead of external).

[jhpratt]

I'm referring to the ::inner being present, not the crate name. A downstream user presumably doesn't care if it's restricted to the crate or something within the crate; they can't implement it regardless.

Not sure it's the best of ideas, but as it's simply a String this could be truncated to just before :: (presumably checking for leading :: as well).

[Urgau]

I don't mind only mentionning the crate name in the main message, as you said it's probably not useful.

It's still nevertheless interesting that the privacy error do mention the other crate modules:

error[E0603]: module `sealed` is private
   --> src/lib.rs:1:17
    |
  1 | trait Foo: std::sealed::Sealed {}
    |                 ^^^^^^  ------ trait `Sealed` is not publicly re-exported
    |                 |
    |                 private module
    |
note: the module `sealed` is defined here
   --> /playground/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/lib.rs:750:1
    |
750 | mod sealed {
    | ^^^^^^^^^^

[jhpratt]

Admittedly that's probably because there currently isn't any better way to point at where something was sealed, where here we are doing so natively.