Hi all, and @pnkfelix (mentioned in one of the issues as someone who might want to collaborate on this). I am drafting an RFC for the continuation of the following:
It's hard to figure out what diagnostics will be convenient to read, so I've included a draft implementation of this feature.
Summary
When types are printed in diagnostics, each type's fully qualified type incurs a cognitive load on the reader. For example, printing the type
std::vec::Vec<std::vec::Vec<std::vec::Vec<u16>>> could have been shortened to Vec<Vec<Vec<u16>>>. This proposal seeks to remedy the situation, by omitted the type's path when possible, however it also seeks to prevent ambiguity.
Motivation
The expected outcome is that it will be easier to read type errors, but without losing the ability to pin-point the exact problem to the user. Currently, a fully qualified type path precisely indicates a type, and we need to make sure that stripping the path does not create ambiguity.
Guide-level explanation
First, for let's define how each non-qualified type falls into either of three categories:
- Types that are imported by default from prelude.
- Types that are imported in the scope in which the type errors occurs, and are not prelude types.
- All other types that don't fall into the first two categories.
In the most reduced from of types printing setting named minimal, we differentiate between the three categories.
- For prelude types, as long as the type is not occluded by an import with the same name, we omitted the path entirely.
- For imports that are available in the current scope, we also omit the type path, but introduce a
via importsline to the type error (see below), illustrative of theusestatement that imports the type. - For all other types, as long as the type is not occluded by an import with the same name, we omit the path, but also introduce a similar
reachable byline to the error message.
Consider the following example:
fn test3(v: &Vec<Vec<Vec<u16>>>) {}
fn main() {
use std::collections::HashMap;
let ip: std::net::IpAddr = "0.0.0.0".parse().unwrap();
test3(vec![vec![(String::from("x"), ip, HashMap::new())]]);
}
The output will be:
7 | test3(vec![vec![(String::from("x"), ip, HashMap::new())]]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected reference, found struct `Vec`
|
= note: expected type `&Vec<Vec<Vec<u16>>>`
found type `Vec<Vec<(String, IpAddr, HashMap<_, _>)>>`
via imports `use std::collections::HashMap;`
reachable by `use std::net::IpAddr;`
Instead of the uniform, default setting thus far:
--> src/case2.rs:7:11
|
7 | test3(vec![vec![(String::from("x"), ip, HashMap::new())]]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected reference, found struct `std::vec::Vec`
|
= note: expected type `&std::vec::Vec<std::vec::Vec<std::vec::Vec<u16>>>`
found type `std::vec::Vec<std::vec::Vec<(std::string::String, std::net::IpAddr, std::collections::HashMap<_, _>)>>`
For the transition period, there is also a by-import setting that limits reduction of type paths for imports, preventing the the reachable by segment.
7 | test3(vec![vec![(String::from("x"), ip, HashMap::new())]]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected reference, found struct `Vec`
|
= note: expected type `&Vec<Vec<Vec<u16>>>`
found type `Vec<Vec<(String, std::net::IpAddr, HashMap<_, _>)>>`
via imports `use std::collections::HashMap;`
Also for the transition period, the three settings can be controlled via a crate-level type_diagnostic feature. For example:
#![feature(type_diagnostic)]
#![type_diagnostic = "minimal"]
More examples in a very early draft rustc implementation of the RFC.
Reference-level explanation
The feature will be implemented via the following steps:
- Produce an
importsmap fromlibrustc_resolveback tolibrustc. - The imports will be a map from the module concept of
resolveto theDefIdthat are defined in that scope, with their respectiveIdents. - The parent scope will be included and walkable from each
HirIdinlibrustc/ty/print/pretty.rs, so gathering up theusesin scope will be efficient. - Code paths that emit errors will need to pass the
HirIdof the relevant scope, which will be stored in TLS forpretty.rsto catch.
Very early implementation draft in rustc.
(TBD: corner cases)
Drawbacks
We may choose not to do this because people are already too accustomed to groking the current and long uniform paths.
Rationale and alternatives
One alternative is that instead of by-import line, we quote the actual span
of the use statement that brought the type. However this may be hard to read,
and the use lines may be too numerous. Also, this does not apply to
reachable via.
The impact of not doing this is a newcomer's roadblock issue, as was already realized in previous discussions.
Prior art
(TBD: needs research, am not aware of other languages doing something like this)
Unresolved questions
- How those prints should be formatted exactly, to make them most readable?
- How do we modify the code for it? Is TLS an
okaysolution or should we do a fuller rewrite of the pretty-printing engine? - Can or should we rely on
Spans instead ofHirId? - Should the default for the setting be kept
uniform, should we change the default, or get rid of the setting altogether?
Future possibilities
From an implementation standpoint, the import_map can be used by analysis
tools for cross-crate important analysis, say, for determining if there is dead
code in a set of packages comprising a workspace.
Also, tools such as rustfix may use the extra printed lines to augment the
use pseudo-section of a module.
