Create Ty type alias in rustc_type_ir

[Jamesbarford]

r? lcnr

Anywhere that required the use of the trait Ty I used ty::Ty<I> otherwise it should be Ty<I>

[lcnr]

can you also add the lint that we should use Ty without any prefix to the type alias? it should be some attribute on struct Ty, idk if it already works with type aliases.

anyways, please import ty::Ty. There isn't really a good reason for that outside of consistency

[Jamesbarford]

I've tried to do this however I'd need to add another #[rustc_diagnostic_item = "Ty"] which then clashes with the one in rustc_middle. I thought perhaps doing something like #[rustc_diagnostic_item = "IrTy"] may work however it then simply ignored the diagnostic. (I've expanded on this in my comment below)

[lcnr]

👍 nice

please also do so for all uses of <whatever as Interner>::Ty

View changes since this review

[Jamesbarford]

Couple of things I found;

1. For methods like Ty::new_error I've had to prefix it with I::Ty::new_error to prevent the error below, the other option would be to do crate::inherent::Ty::new_error(...).

error[E0283]: type annotations needed
   --> compiler/rustc_type_ir/src/ty_kind/closure.rs:486:40
    |
486 |             ty::ClosureKind::FnOnce => Ty::new_tup_from_iter(
    |                                        ^^ cannot infer type for type parameter `I` declared on the type alias `Ty`
    |
    = note: cannot satisfy `_: interner::Interner`
    = note: associated types cannot be accessed directly on a `trait`, they can only be accessed through a specific `impl`
help: use the fully qualified path to an implementation

2. To be able to access the traits methods like ty.is_fresh(...) I've done;

#[cfg_attr(feature = "nightly", allow(rustc::non_glob_import_of_type_ir_inherent))]
use crate::inherent::Ty as _;

3. To prevent ty::Ty the correct thing to do is;

// in compiler/rustc_type_ir/src/sty/mod.rs
#[rustc_diagnostic_item = "Ty"]
pub type Ty ...

// in compiler/rustc_type_ir/src/lib.rs
#![deny(rustc::usage_of_qualified_ty)]

However I then get the below error, so I've not added it. But we should add it when we delete the struct from rustc_middle.

error: duplicate diagnostic item in crate `rustc_middle`: `Ty`
   --> compiler/rustc_middle/src/ty/mod.rs:431:1
    |
431 | pub struct Ty<'tcx>(Interned<'tcx, WithCachedTypeInfo<TyKind<'tcx>>>);
    | ^^^^^^^^^^^^^^^^^^^
    |
    = note: the diagnostic item is first defined in crate `rustc_type_ir`

error: could not compile `rustc_middle` (lib) due to 1 previous error

4. In compiler/rustc_type_ir/src/inherent.rs I import Ty as IrTy to prevent clashing with the trait definition. Seeing as we hope to delete the trait I didn't think it was too bad.

[lcnr]

That is really annoying, didn't consider that... I guess let's keep ty::Ty for now 🤔 this does mean any future PR will have to remove the ty:: at that point, but that should be a single (probably even separate commit which just does a find-and-replace)

[Jamesbarford]

That is really annoying, didn't consider that... I guess let's keep ty::Ty for now 🤔 this does mean any future PR will have to remove the ty:: at that point, but that should be a single (probably even separate commit which just does a find-and-replace)

👍 b95801f

[lcnr]

prefer just not importing Ty and using ty::Ty<I> i think

[lcnr]

final nit, then r=me

View changes since this review

[Jamesbarford]

@bors r=lcnr

[rust-bors]

@Jamesbarford: 🔑 Insufficient privileges: not in review users

[lcnr]

@bors r+ rollup

[rust-bors]

📌 Commit 17e0cc3 has been approved by lcnr

It is now in the queue for this repository.