Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve diagnostics for GATs #82272

Merged
merged 1 commit into from
May 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions compiler/rustc_ast_lowering/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
param_mode: ParamMode,
mut itctx: ImplTraitContext<'_, 'hir>,
) -> hir::QPath<'hir> {
debug!("lower_qpath(id: {:?}, qself: {:?}, p: {:?})", id, qself, p);
let qself_position = qself.as_ref().map(|q| q.position);
let qself = qself.as_ref().map(|q| self.lower_ty(&q.ty, itctx.reborrow()));

Expand Down Expand Up @@ -222,6 +223,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
itctx: ImplTraitContext<'_, 'hir>,
explicit_owner: Option<NodeId>,
) -> hir::PathSegment<'hir> {
debug!(
"path_span: {:?}, lower_path_segment(segment: {:?}, expected_lifetimes: {:?})",
path_span, segment, expected_lifetimes
);
let (mut generic_args, infer_args) = if let Some(ref generic_args) = segment.args {
let msg = "parenthesized type parameters may only be used with a `Fn` trait";
match **generic_args {
Expand Down
14 changes: 14 additions & 0 deletions compiler/rustc_middle/src/middle/resolve_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,20 @@ pub enum Region {
Free(DefId, /* lifetime decl */ DefId),
}

/// This is used in diagnostics to improve suggestions for missing generic arguments.
/// It gives information on the type of lifetimes that are in scope for a particular `PathSegment`,
/// so that we can e.g. suggest elided-lifetimes-in-paths of the form <'_, '_> e.g.
#[derive(Clone, PartialEq, Eq, Hash, TyEncodable, TyDecodable, Debug, HashStable)]
pub enum LifetimeScopeForPath {
// Contains all lifetime names that are in scope and could possibly be used in generics
// arguments of path.
NonElided(Vec<String>),

// Information that allows us to suggest args of the form `<'_>` in case
// no generic arguments were provided for a path.
Elided,
}

/// A set containing, at most, one known element.
/// If two distinct values are inserted into a set, then it
/// becomes `Many`, which can be used to detect ambiguities.
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1301,6 +1301,10 @@ rustc_queries! {
desc { "looking up late bound vars" }
}

query lifetime_scope_map(_: LocalDefId) -> Option<FxHashMap<ItemLocalId, LifetimeScopeForPath>> {
desc { "finds the lifetime scope for an HirId of a PathSegment" }
}

query visibility(def_id: DefId) -> ty::Visibility {
eval_always
desc { |tcx| "computing visibility of `{}`", tcx.def_path_str(def_id) }
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::infer::canonical::{Canonical, CanonicalVarInfo, CanonicalVarInfos};
use crate::lint::{struct_lint_level, LintDiagnosticBuilder, LintLevelSource};
use crate::middle;
use crate::middle::cstore::{CrateStoreDyn, EncodedMetadata};
use crate::middle::resolve_lifetime::{self, ObjectLifetimeDefault};
use crate::middle::resolve_lifetime::{self, LifetimeScopeForPath, ObjectLifetimeDefault};
use crate::middle::stability;
use crate::mir::interpret::{self, Allocation, ConstValue, Scalar};
use crate::mir::{Body, Field, Local, Place, PlaceElem, ProjectionKind, Promoted};
Expand Down Expand Up @@ -2686,6 +2686,10 @@ impl<'tcx> TyCtxt<'tcx> {
.iter(),
)
}

pub fn lifetime_scope(self, id: HirId) -> Option<LifetimeScopeForPath> {
self.lifetime_scope_map(id.owner).and_then(|mut map| map.remove(&id.local_id))
}
}

impl TyCtxtAt<'tcx> {
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_middle/src/ty/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ use crate::middle::exported_symbols::{ExportedSymbol, SymbolExportLevel};
use crate::middle::lib_features::LibFeatures;
use crate::middle::privacy::AccessLevels;
use crate::middle::region;
use crate::middle::resolve_lifetime::{ObjectLifetimeDefault, Region, ResolveLifetimes};
use crate::middle::resolve_lifetime::{
LifetimeScopeForPath, ObjectLifetimeDefault, Region, ResolveLifetimes,
};
use crate::middle::stability::{self, DeprecationEntry};
use crate::mir;
use crate::mir::interpret::GlobalId;
Expand Down
132 changes: 114 additions & 18 deletions compiler/rustc_resolve/src/late/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@

use crate::late::diagnostics::{ForLifetimeSpanType, MissingLifetimeSpot};
use rustc_ast::walk_list;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder};
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefIdMap;
use rustc_hir::def_id::{DefIdMap, LocalDefId};
use rustc_hir::hir_id::ItemLocalId;
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_hir::{GenericArg, GenericParam, LifetimeName, Node, ParamName, QPath};
Expand All @@ -22,7 +22,7 @@ use rustc_middle::middle::resolve_lifetime::*;
use rustc_middle::ty::{self, DefIdTree, GenericParamDefKind, TyCtxt};
use rustc_middle::{bug, span_bug};
use rustc_session::lint;
use rustc_span::def_id::{DefId, LocalDefId};
use rustc_span::def_id::DefId;
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::Span;
use std::borrow::Cow;
Expand Down Expand Up @@ -158,6 +158,9 @@ struct NamedRegionMap {
// - trait refs
// - bound types (like `T` in `for<'a> T<'a>: Foo`)
late_bound_vars: HirIdMap<Vec<ty::BoundVariableKind>>,

// maps `PathSegment` `HirId`s to lifetime scopes.
scope_for_path: Option<FxHashMap<LocalDefId, FxHashMap<ItemLocalId, LifetimeScopeForPath>>>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we aren't going to be caching this anyways, we should just use a HirIdMap directly.

}

crate struct LifetimeContext<'a, 'tcx> {
Expand Down Expand Up @@ -195,7 +198,9 @@ enum Scope<'a> {
/// it should be shifted by the number of `Binder`s in between the
/// declaration `Binder` and the location it's referenced from.
Binder {
lifetimes: FxHashMap<hir::ParamName, Region>,
/// We use an IndexMap here because we want these lifetimes in order
/// for diagnostics.
lifetimes: FxIndexMap<hir::ParamName, Region>,

/// if we extend this scope with another scope, what is the next index
/// we should use for an early-bound region?
Expand Down Expand Up @@ -379,6 +384,10 @@ pub fn provide(providers: &mut ty::query::Providers) {
}
},
late_bound_vars_map: |tcx, id| resolve_lifetimes_for(tcx, id).late_bound_vars.get(&id),
lifetime_scope_map: |tcx, id| {
let item_id = item_for(tcx, id);
do_resolve(tcx, item_id, false, true).scope_for_path.unwrap().remove(&id)
},

..*providers
};
Expand Down Expand Up @@ -419,27 +428,29 @@ fn resolve_lifetimes_trait_definition(
tcx: TyCtxt<'_>,
local_def_id: LocalDefId,
) -> ResolveLifetimes {
do_resolve(tcx, local_def_id, true)
convert_named_region_map(do_resolve(tcx, local_def_id, true, false))
}

/// Computes the `ResolveLifetimes` map that contains data for an entire `Item`.
/// You should not read the result of this query directly, but rather use
/// `named_region_map`, `is_late_bound_map`, etc.
#[tracing::instrument(level = "debug", skip(tcx))]
fn resolve_lifetimes(tcx: TyCtxt<'_>, local_def_id: LocalDefId) -> ResolveLifetimes {
do_resolve(tcx, local_def_id, false)
convert_named_region_map(do_resolve(tcx, local_def_id, false, false))
}

fn do_resolve(
tcx: TyCtxt<'_>,
local_def_id: LocalDefId,
trait_definition_only: bool,
) -> ResolveLifetimes {
with_scope_for_path: bool,
) -> NamedRegionMap {
let item = tcx.hir().expect_item(tcx.hir().local_def_id_to_hir_id(local_def_id));
let mut named_region_map = NamedRegionMap {
defs: Default::default(),
late_bound: Default::default(),
late_bound_vars: Default::default(),
scope_for_path: with_scope_for_path.then(|| Default::default()),
};
let mut visitor = LifetimeContext {
tcx,
Expand All @@ -455,6 +466,10 @@ fn do_resolve(
};
visitor.visit_item(item);

named_region_map
}

fn convert_named_region_map(named_region_map: NamedRegionMap) -> ResolveLifetimes {
let mut rl = ResolveLifetimes::default();

for (hir_id, v) in named_region_map.defs {
Expand Down Expand Up @@ -567,6 +582,41 @@ fn late_region_as_bound_region<'tcx>(tcx: TyCtxt<'tcx>, region: &Region) -> ty::
}
}

#[tracing::instrument(level = "debug")]
fn get_lifetime_scopes_for_path(mut scope: &Scope<'_>) -> LifetimeScopeForPath {
let mut available_lifetimes = vec![];
loop {
match scope {
Scope::Binder { lifetimes, s, .. } => {
available_lifetimes.extend(lifetimes.keys().filter_map(|p| match p {
hir::ParamName::Plain(ident) => Some(ident.name.to_string()),
_ => None,
}));
scope = s;
}
Scope::Body { s, .. } => {
scope = s;
}
Scope::Elision { elide, s } => {
if let Elide::Exact(_) = elide {
return LifetimeScopeForPath::Elided;
} else {
scope = s;
}
}
Scope::ObjectLifetimeDefault { s, .. } => {
scope = s;
}
Scope::Root => {
return LifetimeScopeForPath::NonElided(available_lifetimes);
}
Scope::Supertrait { s, .. } | Scope::TraitRefBoundary { s, .. } => {
scope = s;
}
}
}
}

impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
/// Returns the binders in scope and the type of `Binder` that should be created for a poly trait ref.
fn poly_trait_ref_binder_info(&mut self) -> (Vec<ty::BoundVariableKind>, BinderScopeType) {
Expand Down Expand Up @@ -656,7 +706,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
self.map.late_bound_vars.insert(hir_id, vec![]);
let scope = Scope::Binder {
hir_id,
lifetimes: FxHashMap::default(),
lifetimes: FxIndexMap::default(),
next_early_index: self.next_early_index(),
s: self.scope,
track_lifetime_uses: true,
Expand Down Expand Up @@ -720,9 +770,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
// We need to add *all* deps, since opaque tys may want them from *us*
for (&owner, defs) in resolved_lifetimes.defs.iter() {
defs.iter().for_each(|(&local_id, region)| {
self.map
.defs
.insert(hir::HirId { owner, local_id }, region.clone());
self.map.defs.insert(hir::HirId { owner, local_id }, *region);
});
}
for (&owner, late_bound) in resolved_lifetimes.late_bound.iter() {
Expand Down Expand Up @@ -836,7 +884,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
};
self.missing_named_lifetime_spots
.push(MissingLifetimeSpot::HigherRanked { span, span_type });
let (lifetimes, binders): (FxHashMap<hir::ParamName, Region>, Vec<_>) = c
let (lifetimes, binders): (FxIndexMap<hir::ParamName, Region>, Vec<_>) = c
.generic_params
.iter()
.filter_map(|param| match param.kind {
Expand Down Expand Up @@ -1010,7 +1058,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
debug!(?index);

let mut elision = None;
let mut lifetimes = FxHashMap::default();
let mut lifetimes = FxIndexMap::default();
let mut non_lifetime_count = 0;
for param in generics.params {
match param.kind {
Expand Down Expand Up @@ -1181,7 +1229,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
let mut index = self.next_early_index();
let mut non_lifetime_count = 0;
debug!("visit_ty: index = {}", index);
let lifetimes: FxHashMap<hir::ParamName, Region> = generics
let lifetimes: FxIndexMap<hir::ParamName, Region> = generics
.params
.iter()
.filter_map(|param| match param.kind {
Expand Down Expand Up @@ -1241,15 +1289,53 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
self.resolve_lifetime_ref(lifetime_ref);
}

fn visit_assoc_type_binding(&mut self, type_binding: &'tcx hir::TypeBinding<'_>) {
let scope = self.scope;
if let Some(scope_for_path) = self.map.scope_for_path.as_mut() {
// We add lifetime scope information for `Ident`s in associated type bindings and use
// the `HirId` of the type binding as the key in `LifetimeMap`
let lifetime_scope = get_lifetime_scopes_for_path(scope);
let map = scope_for_path.entry(type_binding.hir_id.owner).or_default();
map.insert(type_binding.hir_id.local_id, lifetime_scope);
}
hir::intravisit::walk_assoc_type_binding(self, type_binding);
}

fn visit_path(&mut self, path: &'tcx hir::Path<'tcx>, _: hir::HirId) {
for (i, segment) in path.segments.iter().enumerate() {
let depth = path.segments.len() - i - 1;
if let Some(ref args) = segment.args {
self.visit_segment_args(path.res, depth, args);
}

let scope = self.scope;
if let Some(scope_for_path) = self.map.scope_for_path.as_mut() {
// Add lifetime scope information to path segment. Note we cannot call `visit_path_segment`
// here because that call would yield to resolution problems due to `walk_path_segment`
// being called, which processes the path segments generic args, which we have already
// processed using `visit_segment_args`.
let lifetime_scope = get_lifetime_scopes_for_path(scope);
if let Some(hir_id) = segment.hir_id {
let map = scope_for_path.entry(hir_id.owner).or_default();
map.insert(hir_id.local_id, lifetime_scope);
}
}
}
}

fn visit_path_segment(&mut self, path_span: Span, path_segment: &'tcx hir::PathSegment<'tcx>) {
let scope = self.scope;
if let Some(scope_for_path) = self.map.scope_for_path.as_mut() {
jackh726 marked this conversation as resolved.
Show resolved Hide resolved
let lifetime_scope = get_lifetime_scopes_for_path(scope);
if let Some(hir_id) = path_segment.hir_id {
let map = scope_for_path.entry(hir_id.owner).or_default();
map.insert(hir_id.local_id, lifetime_scope);
}
}

intravisit::walk_path_segment(self, path_span, path_segment);
}

fn visit_fn_decl(&mut self, fd: &'tcx hir::FnDecl<'tcx>) {
let output = match fd.output {
hir::FnRetTy::DefaultReturn(_) => None,
Expand Down Expand Up @@ -1290,7 +1376,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
ref bound_generic_params,
..
}) => {
let (lifetimes, binders): (FxHashMap<hir::ParamName, Region>, Vec<_>) =
let (lifetimes, binders): (FxIndexMap<hir::ParamName, Region>, Vec<_>) =
bound_generic_params
.iter()
.filter_map(|param| match param.kind {
Expand Down Expand Up @@ -1360,7 +1446,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
self.map.late_bound_vars.insert(*hir_id, binders);
let scope = Scope::Binder {
hir_id: *hir_id,
lifetimes: FxHashMap::default(),
lifetimes: FxIndexMap::default(),
s: self.scope,
next_early_index: self.next_early_index(),
track_lifetime_uses: true,
Expand Down Expand Up @@ -1388,7 +1474,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
let (mut binders, scope_type) = self.poly_trait_ref_binder_info();

let initial_bound_vars = binders.len() as u32;
let mut lifetimes: FxHashMap<hir::ParamName, Region> = FxHashMap::default();
let mut lifetimes: FxIndexMap<hir::ParamName, Region> = FxIndexMap::default();
let binders_iter = trait_ref
.bound_generic_params
.iter()
Expand Down Expand Up @@ -2115,7 +2201,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {

let mut non_lifetime_count = 0;
let mut named_late_bound_vars = 0;
let lifetimes: FxHashMap<hir::ParamName, Region> = generics
let lifetimes: FxIndexMap<hir::ParamName, Region> = generics
.params
.iter()
.filter_map(|param| match param.kind {
Expand Down Expand Up @@ -3034,6 +3120,16 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
}
};

// If we specifically need the `scope_for_path` map, then we're in the
// diagnostic pass and we don't want to emit more errors.
if self.map.scope_for_path.is_some() {
estebank marked this conversation as resolved.
Show resolved Hide resolved
self.tcx.sess.delay_span_bug(
rustc_span::DUMMY_SP,
"Encountered unexpected errors during diagnostics related part",
);
return;
}

let mut spans: Vec<_> = lifetime_refs.iter().map(|lt| lt.span).collect();
spans.sort();
let mut spans_dedup = spans.clone();
Expand Down
Loading