search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
99.03k stars 12.79k forks source link

consistently elide/don't elide types in diagnostics #118731

Open jyn514 opened 11 months ago

jyn514 commented 11 months ago

right now, the decision of whether to fully elide the type as _ is inconsistent - it never happens for functions, fn pointers, tuples, refs where the pointee types are equal, or adts, but it does happen for primitives and ADTs behind a raw pointer. that is probably not intended? might be worth moving the t1 == t2 comparison up above the big match for consistency. https://github.com/rust-lang/rust/blob/eb53721a34d1d910b900aa753e0a00dc72ef41ac/compiler/rustc_infer/src/infer/error_reporting/mod.rs#L1510-L1512

here is an example of it being inconsistent: https://github.com/rust-lang/rust/blob/cf2dff2b1e3fa55fa5415d524200070d0d7aacfe/tests/ui/issues/issue-17905-2.stderr#L23-L26 isize is elided but str is not.

i originally changed this to almost always elide the type, but @estebank is worried that will be confusing: image he suggested instead eliding if the type has either type params or projection types ("looks complicated", basically).

Originally posted by @jyn514 in https://github.com/rust-lang/rust/issues/118730#issuecomment-1846520771

@rustbot label +A-diagnostics +E-medium

Liewyec commented 10 months ago

Hello, I would like to look at this. Is there something I need to be aware of?

estebank commented 10 months ago

@Liewyec take a look at the conversation in the linked PR. The main thing to be careful with here is taking a look at the changes to the .stderr files after running python x.py test tests/ui --bless to verify that you're not accidentally removing useful information. In general, _ should only be shown for types that are not interesting or useful for the user, but for cases like Ty<'a>/Box<Ty<'a>, the lifetimes aren't useful, but knowing that "I have a type, but the same type must be boxed" is easier to understand if the (short) type Ty<'_> here is shown, instead of _/Box<_> which can make people take longer than strictly necessary to understand what the compiler is saying.

Liewyec commented 10 months ago

all right it seems simple enough, I will do this

estebank commented 10 months ago

Be aware that by the nature of this part of the codebase (free-form tree comparisons), the logic can be daunting. Try to understand each chunk independently, particularly by getting a high level understanding first and then digging in, to avoid getting overwhelmed. It's not hard, just a lot of state to hold in your head and can take some time to build up that context.