Update non-local-defns regarding parameterized traits and types

[dhardy]

The existing RFC does not give sufficient attention to parametrized types and traits.

• Rendered
• Original RFC
• Tracking issue

[matklad]

Agree that this is under specified. I am not sure that spelling this out explicitly in the RFC text works --- its not clear whether the specified rules have any loopholes.

My current belief is that the rule we want here is exactly equivalent to coherence. So the RFC should say something along to the effect of "function bodies are treated as downstream crates for the purposes of trait checking and type inference".

As this (most excellent) example by QuineDot demonstrates

https://github.com/rust-lang/rfcs/pull/3373#issuecomment-1416529034

it is important to restrict both:

• which impls are allowed inside
• which inside impls are visible from the outside, for the purpose of type inference reasoning that "I know of a single impl for this trait, so _ must be that type", where "that type" might end up being a type from a function's body.

[matklad]

Ah, I missed this extra bit of context: https://github.com/rust-lang/rust/issues/121621, where it is suggested that we want more restritive rules than coherence here specifically to avoid fixind point two from above.

[ehuss]

@tmandry Is this something you can review (or direct to someone who can)?

I can't tell if this is changing the RFC in a meaningful way, or just clarifying it. If it is a change from before, it would probably be best to start a new RFC.

[tmandry]

From a quick look this is in line with the original RFC, and if @joshtriplett is willing to consider it a "friendly amendment" I think it can be merged.

[joshtriplett]

Made a few editorial changes, and made one comment about moving the example out of the motivation section. With that change made, LGTM; no objections.

[Urgau]

Isn't this going against T-types solution ? (which is being implemented in https://github.com/rust-lang/rust/pull/122747, @lcnr) It was even approved by T-lang.

[joshtriplett]

@Urgau Thanks for catching that. @lcnr, can you help resolve the discrepancy here?

[lcnr]

I'd personally just keep the RFC as "we lint if the existance of the impl is observable outside of its containing item", I don't think there is much value in explaining the exact algorithm used for that in an RFC. I also thought we generally expect the reference/guidelevel explanation of RFCs to get out of date and do not bother updating them, instead referring to the reference/docs.

The idea implemented in https://github.com/rust-lang/rust/pull/122747 is as follows:

Lint impls inside of bodies unless:

• the body is of an unnamed const, or
• the self type is also local to the body, or
• for trait impls: after replacing all generic params and types in the local to the body with inference variables in the "impl trait ref", there are at least 2 global impls which could also prove that trait bound

This should mean that in all cases where the local impl may apply outside of the body, we already fail with ambiguity even when ignoring the impl

[joshtriplett]

@lcnr I'm deferring entirely to the types team here for whether you want to make changes/suggestions to this or close it.

It'd be nice if the rules we have are documented somewhere. Updating the RFC is one possible option; others could work as well.

[dhardy]

whether you want to make changes/suggestions to this or close it

My only feelings is that it is often hard to tell the status and intended behaviour of in-development features. Whether or not updating RFCs is appropriate I don't know... and now we're getting off-topic.