Let the bikeshedding commence.

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

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

I basically like this. I'm not sure about moving all the impls to a single module, but it might be nice. I've been wanting to split ty.rs into a directory for some time, for sure.

@arielb1 this is just a reorganization, right? No behavioral changes?

@nikomatsakis

Right. No behavioral or (intentional) performance changes.

I didn't move all impls, just Lift, HasTypeFlags, RegionEscape and TypeFoldable.

I like this as well 👍, but after 1214, MIR, and HIR this is going to be another "fun" rebase on my branch.

@arielb1

I didn't move all impls, just Lift, HasTypeFlags, RegionEscape and TypeFoldable.

So I guess the interesting question is where these impls should go in general. I've sometimes regretted the current style -- I tend to think the impls should probably go with the types (though it's natural when first defining the traits to stick them with the trait). This is particularly true since in the cross-crate case they must go with the types. Note that "impls go with the types" might be loosely defined -- e.g., all types defined in the ty module might put their impls under ty::impls, but things in traits might be under traits::impls, etc.

r+ from me, in any case. This looks like progress to me. I second the... joyful thoughts of rebases to come.

@nikomatsakis

90% of the impls are for ty and rustc dependencies (std, syntax, etc.), and therefore would go in ty::structural_impls anyway. Should I create a similar traits::structural_impls for these?

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

@arielb1 I think I would prefer that, yes. The Debug impls for traits are already stashed in util.rs, they could be combined together as well (basically, all the... de rigeur impls).

But then I see you already did it.

@bors r+

📌 Commit 5a95acb has been approved by nikomatsakis

⌛ Testing commit 5a95acb with merge b1c9616...

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt

Are InfrigingField and InfrigingVariant not typos?