Allow impl on projection

[GuillaumeGomez]

Got this one from #103985 as well.

Issue (solved) with privacy check

However, a bug appeared from the privacy check as the CI will show: it has some false positive with "private type in public interface". Not sure why though. The backtrace for it is here:

  12:     0x7fef586494c5 - <rustc_privacy[dc2692dda3b059d]::SearchInterfaceForPrivateItemsVisitor as rustc_privacy[dc2692dda3b059d]::DefIdVisitor>::visit_def_id
  13:     0x7fef5863f6ff - <rustc_privacy[dc2692dda3b059d]::DefIdVisitorSkeleton<rustc_privacy[dc2692dda3b059d]::SearchInterfaceForPrivateItemsVisitor> as rustc_middle[2d95345cc9d1265e]::ty::visit::TypeVisitor>::visit_ty
  14:     0x7fef5863f9d2 - <rustc_privacy[dc2692dda3b059d]::DefIdVisitorSkeleton<rustc_privacy[dc2692dda3b059d]::SearchInterfaceForPrivateItemsVisitor> as rustc_middle[2d95345cc9d1265e]::ty::visit::TypeVisitor>::visit_ty
  15:     0x7fef586484c2 - <rustc_privacy[dc2692dda3b059d]::SearchInterfaceForPrivateItemsVisitor>::ty
  16:     0x7fef586497c5 - <rustc_privacy[dc2692dda3b059d]::PrivateItemsInPublicInterfacesChecker>::check_assoc_item
  17:     0x7fef5864bba5 - rustc_privacy[dc2692dda3b059d]::check_private_in_public
  18:     0x7fef59c15a66 - rustc_query_system[a3c6a6fb37b6854f]::query::plumbing::try_execute_query::<rustc_query_impl[eee68f6de69b6b9]::queries::check_private_in_public, rustc_query_impl[eee68f6de69b6b9]::plumbing::QueryCtxt>
  19:     0x7fef598fc292 - <rustc_query_impl[eee68f6de69b6b9]::Queries as rustc_middle[2d95345cc9d1265e]::ty::query::QueryEngine>::check_private_in_public
  20:     0x7fef58179756 - <core[3d15cbc1632fd665]::panic::unwind_safe::AssertUnwindSafe<rustc_interface[2af05e4c67454c01]::passes::analysis::{closure#5}::{closure#1}> as core[3d15cbc1632fd665]::ops::function::FnOnce<()>>::call_once
  21:     0x7fef580e26c5 - <rustc_session[da714fc9e163a7bd]::session::Session>::time::<(), rustc_interface[2af05e4c67454c01]::passes::analysis::{closure#5}>
  22:     0x7fef580b9dac - rustc_interface[2af05e4c67454c01]::passes::analysis
  23:     0x7fef59c644b2 - rustc_query_system[a3c6a6fb37b6854f]::query::plumbing::try_execute_query::<rustc_query_impl[eee68f6de69b6b9]::queries::analysis, rustc_query_impl[eee68f6de69b6b9]::plumbing::QueryCtxt>
  24:     0x7fef598cfb95 - <rustc_query_impl[eee68f6de69b6b9]::Queries as rustc_middle[2d95345cc9d1265e]::ty::query::QueryEngine>::analysis
  25:     0x7fef57f91c03 - <rustc_interface[2af05e4c67454c01]::passes::QueryContext>::enter::<rustc_driver[de5d577c9fbc1a01]::run_compiler::{closure#1}::{closure#2}::{closure#2}, core[3d15cbc1632fd665]::result::Result<(), rustc_errors[c6c462034af138b3]::ErrorGuaranteed>>
  26:     0x7fef57f7383a - <rustc_interface[2af05e4c67454c01]::queries::QueryResult<rustc_interface[2af05e4c67454c01]::passes::QueryContext>>::enter::<core[3d15cbc1632fd665]::result::Result<(), rustc_errors[c6c462034af138b3]::ErrorGuaranteed>, rustc_driver[de5d577c9fbc1a01]::run_compiler::{closure#1}::{closure#2}::{closure#2}>
  27:     0x7fef57fddc76 - <rustc_interface[2af05e4c67454c01]::interface::Compiler>::enter::<rustc_driver[de5d577c9fbc1a01]::run_compiler::{closure#1}::{closure#2}, core[3d15cbc1632fd665]::result::Result<core[3d15cbc1632fd665]::option::Option<rustc_interface[2af05e4c67454c01]::queries::Linker>, rustc_errors[c6c462034af138b3]::ErrorGuaranteed>>
  28:     0x7fef5800406a - rustc_span[29a73c65a118f2f3]::with_source_map::<core[3d15cbc1632fd665]::result::Result<(), rustc_errors[c6c462034af138b3]::ErrorGuaranteed>, rustc_interface[2af05e4c67454c01]::interface::run_compiler<core[3d15cbc1632fd665]::result::Result<(), rustc_errors[c6c462034af138b3]::ErrorGuaranteed>, rustc_driver[de5d577c9fbc1a01]::run_compiler::{closure#1}>::{closure#0}::{closure#0}>
  29:     0x7fef57fd44c5 - <scoped_tls[535ca24a62cbd8c4]::ScopedKey<rustc_span[29a73c65a118f2f3]::SessionGlobals>>::set::<rustc_interface[2af05e4c67454c01]::interface::run_compiler<core[3d15cbc1632fd665]::result::Result<(), rustc_errors[c6c462034af138b3]::ErrorGuaranteed>, rustc_driver[de5d577c9fbc1a01]::run_compiler::{closure#1}>::{closure#0}, core[3d15cbc1632fd665]::result::Result<(), rustc_errors[c6c462034af138b3]::ErrorGuaranteed>>
  30:     0x7fef57f9ad30 - std[1177681641634ed6]::sys_common::backtrace::__rust_begin_short_backtrace::<rustc_interface[2af05e4c67454c01]::util::run_in_thread_pool_with_globals<rustc_interface[2af05e4c67454c01]::interface::run_compiler<core[3d15cbc1632fd665]::result::Result<(), rustc_errors[c6c462034af138b3]::ErrorGuaranteed>, rustc_driver[de5d577c9fbc1a01]::run_compiler::{closure#1}>::{closure#0}, core[3d15cbc1632fd665]::result::Result<(), rustc_errors[c6c462034af138b3]::ErrorGuaranteed>>::{closure#0}::{closure#0}, core[3d15cbc1632fd665]::result::Result<(), rustc_errors[c6c462034af138b3]::ErrorGuaranteed>>
  31:     0x7fef57f7bdbd - <<std[1177681641634ed6]::thread::Builder>::spawn_unchecked_<rustc_interface[2af05e4c67454c01]::util::run_in_thread_pool_with_globals<rustc_interface[2af05e4c67454c01]::interface::run_compiler<core[3d15cbc1632fd665]::result::Result<(), rustc_errors[c6c462034af138b3]::ErrorGuaranteed>, rustc_driver[de5d577c9fbc1a01]::run_compiler::{closure#1}>::{closure#0}, core[3d15cbc1632fd665]::result::Result<(), rustc_errors[c6c462034af138b3]::ErrorGuaranteed>>::{closure#0}::{closure#0}, core[3d15cbc1632fd665]::result::Result<(), rustc_errors[c6c462034af138b3]::ErrorGuaranteed>>::{closure#1} as core[3d15cbc1632fd665]::ops::function::FnOnce<()>>::call_once::{shim:vtable#0}

cc @compiler-errors
r? @oli-obk

[GuillaumeGomez]

Fixed the privacy bug. With this it should be complete.

[GuillaumeGomez]

I added another UI tests for implementations on foreign types (not primitive types).

[compiler-errors]

Sorry, this is what I wanted to mention. I'm not sure if we want to be able to normalize impl headers when considering inherent impls.

[GuillaumeGomez]

That's also something I wondered but after talking with @oli-obk, they seemed to think it was ok so I made this PR. I think it'll require an FCP in any case so you can see with your team whether or not it's something you want.

[oli-obk]

My words were "why should we not allow it", I don't know the answer to that. Mostly I don't know the use case for it

[GuillaumeGomez]

Sorry for the confusion. Well in any case, I'll let your team discuss about it.

[sdroege]

I remember having written code exactly like that before and then being surprised that it didn't compile. I can't remember where that was and I must've found a different approach, but it seems like something that is consistent with how other parts of the language works. And it's kind of surprising/unexpected that it doesn't compile.

[oli-obk]

I guess this should either specifically check for projections or not have any check at all. The logic in rustc_privacy should be kept in sync with this

[GuillaumeGomez]

If I remove the check, I have this panic:

error[E0391]: cycle detected when finding all inherent impls defined in crate
   |
   = note: ...which requires normalizing `I8<{i8::MIN}>`...
note: ...which requires evaluating type-level constant...
  --> fake-test-src-base/symbol-names/const-generics.rs:20:9
   |
LL | impl I8<{i8::MIN}> {
   |         ^^^^^^^^^

Also, doesn't it make sense to normalize it for type aliases too?

And final question: you mean that if we keep this check, we should have the same in privacy where I added it (just to confirm so I can update accordingly).

[oli-obk]

If I remove the check, I have this panic:

that's an error, and I think it can be avoided by using the try_normalize variant, just like at the other normalization added in this PR. The check you are doing is useless if the projection resolves to a type that would have errored without your check (as normalization is recursive/deep)

Also, doesn't it make sense to normalize it for type aliases too?

there are no type aliases in the type system yet ;) that's your other PR

you mean that if we keep this check, we should have the same in privacy where I added it (just to confirm so I can update accordingly).

I just mean that they should work as similarly as possible, so if both use the try variant, that seems okay.

[petrochenkov]

Could you assign this PR to me when it's otherwise ready?
The relevant logic in rustc_privacy is subtle and I need to refresh it in memory and check whether it's still correct.

[GuillaumeGomez]

So we discovered that the normalization bug also appeared on this code:

pub trait Identity {
    type Identity: ?Sized;
}

impl<T: ?Sized> Identity for T {
    type Identity = Self;
}

pub struct I8<const F: i8>;

impl <I8<{i8::MIN}> as Identity>::Identity {
    pub fn foo(&self) {}
}

meaning that the if matches!(...) condition is completely useless and that the normalization bug should be fixed instead. Before digging into it, I'll wait to know if the team wants this feature or not.

[GuillaumeGomez]

We discovered a use case in gtk-rs.

#[properties(wrapper_type = super::Foo)] is only needed so we can do impl super::Foo { ... }. Ideally we would just do impl <Foo as glib::ObjectSubclass>::Type { ... } from this code:

https://gtk-rs.org/gtk-rs-core/git/docs/glib_macros/derive.Properties.html#example

cc @sdroege

[bors]

☔ The latest upstream changes (presumably #108015) made this pull request unmergeable. Please resolve the merge conflicts.

[lcnr]

going to talk about this in a types team deep dive rust-lang/types-team#91