Skip to content

Commit

Permalink
Rollup merge of #52168 - nikomatsakis:nll-region-name, r=estebank
Browse files Browse the repository at this point in the history
find and highlight the `&` or `'_` in `region_name`

Before:

```
   --> $DIR/dyn-trait-underscore.rs:18:5
    |
 LL | fn a<T>(items: &[T]) -> Box<dyn Iterator<Item=&T>> {
-   |         ----- lifetime `'1` appears in this argument
 LL |     Box::new(items.iter()) //~ ERROR cannot infer an appropriate lifetime
    |     ^^^^^^^^^^^^^^^^^^^^^^ cast requires that `'1` must outlive `'static`
```

After:

```
   --> $DIR/dyn-trait-underscore.rs:18:5
    |
 LL | fn a<T>(items: &[T]) -> Box<dyn Iterator<Item=&T>> {
+   |                - let's call the lifetime of this reference `'1`
 LL |     Box::new(items.iter()) //~ ERROR cannot infer an appropriate lifetime
    |     ^^^^^^^^^^^^^^^^^^^^^^ cast requires that `'1` must outlive `'static`
```

Not intended as the final end point necessarily in any sense. I intentionally left some to-do points to fill in later:

- Does not apply to upvars in closures yet (should be relatively easy)
- Does not handle the case where we can't find a precise match very well
- And of course we can still tweak wording

but shows the basic idea of how to make the `Ty` and `hir::Ty` to find a good spot to highlight.

r? @estebank
cc @davidtwco
  • Loading branch information
Mark-Simulacrum committed Jul 10, 2018
2 parents f2a26ea + a6adb1e commit fa9f580
Show file tree
Hide file tree
Showing 7 changed files with 300 additions and 22 deletions.
9 changes: 9 additions & 0 deletions src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,15 @@ pub enum GenericArg {
Type(Ty),
}

impl GenericArg {
pub fn span(&self) -> Span {
match self {
GenericArg::Lifetime(l) => l.span,
GenericArg::Type(t) => t.span,
}
}
}

#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
pub struct GenericArgs {
/// The generic arguments for this path segment.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@

use borrow_check::nll::region_infer::RegionInferenceContext;
use borrow_check::nll::ToRegionVid;
use rustc::hir;
use rustc::hir::def_id::DefId;
use rustc::mir::{Local, Mir};
use rustc::ty::{self, RegionVid, TyCtxt};
use rustc::ty::subst::{Substs, UnpackedKind};
use rustc::ty::{self, RegionVid, Ty, TyCtxt};
use rustc_data_structures::indexed_vec::Idx;
use rustc_errors::DiagnosticBuilder;
use syntax::ast::Name;
Expand Down Expand Up @@ -60,7 +62,9 @@ impl<'tcx> RegionInferenceContext<'tcx> {

self.give_name_from_error_region(tcx, mir_def_id, fr, counter, diag)
.or_else(|| {
self.give_name_if_anonymous_region_appears_in_arguments(tcx, mir, fr, counter, diag)
self.give_name_if_anonymous_region_appears_in_arguments(
tcx, mir, mir_def_id, fr, counter, diag,
)
})
.or_else(|| {
self.give_name_if_anonymous_region_appears_in_upvars(tcx, mir, fr, counter, diag)
Expand Down Expand Up @@ -129,30 +133,48 @@ impl<'tcx> RegionInferenceContext<'tcx> {
&self,
tcx: TyCtxt<'_, '_, 'tcx>,
mir: &Mir<'tcx>,
mir_def_id: DefId,
fr: RegionVid,
counter: &mut usize,
diag: &mut DiagnosticBuilder<'_>,
) -> Option<InternedString> {
let implicit_inputs = self.universal_regions.defining_ty.implicit_inputs();
let argument_index = self.universal_regions
let argument_index = self
.universal_regions
.unnormalized_input_tys
.iter()
.skip(implicit_inputs)
.position(|arg_ty| {
debug!("give_name_if_anonymous_region_appears_in_arguments: arg_ty = {:?}", arg_ty);
debug!(
"give_name_if_anonymous_region_appears_in_arguments: arg_ty = {:?}",
arg_ty
);
tcx.any_free_region_meets(arg_ty, |r| r.to_region_vid() == fr)
})?
+ implicit_inputs;
})?;

debug!(
"give_name_if_anonymous_region_appears_in_arguments: \
found {:?} in argument {} which has type {:?}",
fr, argument_index, self.universal_regions.unnormalized_input_tys[argument_index],
);

let arg_ty =
self.universal_regions.unnormalized_input_tys[implicit_inputs + argument_index];
if let Some(region_name) = self.give_name_if_we_can_match_hir_ty_from_argument(
tcx,
mir_def_id,
fr,
arg_ty,
argument_index,
counter,
diag,
) {
return Some(region_name);
}

let region_name = self.synthesize_region_name(counter);

let argument_local = Local::new(argument_index + 1);
let argument_local = Local::new(argument_index + implicit_inputs + 1);
let argument_span = mir.local_decls[argument_local].source_info.span;
diag.span_label(
argument_span,
Expand All @@ -162,6 +184,240 @@ impl<'tcx> RegionInferenceContext<'tcx> {
Some(region_name)
}

fn give_name_if_we_can_match_hir_ty_from_argument(
&self,
tcx: TyCtxt<'_, '_, 'tcx>,
mir_def_id: DefId,
needle_fr: RegionVid,
argument_ty: Ty<'tcx>,
argument_index: usize,
counter: &mut usize,
diag: &mut DiagnosticBuilder<'_>,
) -> Option<InternedString> {
let mir_node_id = tcx.hir.as_local_node_id(mir_def_id)?;
let fn_decl = tcx.hir.fn_decl(mir_node_id)?;
let argument_hir_ty: &hir::Ty = &fn_decl.inputs[argument_index];
match argument_hir_ty.node {
// This indicates a variable with no type annotation, like
// `|x|`... in that case, we can't highlight the type but
// must highlight the variable.
hir::TyInfer => None,

_ => self.give_name_if_we_can_match_hir_ty(
tcx,
needle_fr,
argument_ty,
argument_hir_ty,
counter,
diag,
),
}
}

/// Attempts to highlight the specific part of a type annotation
/// that contains the anonymous reference we want to give a name
/// to. For example, we might produce an annotation like this:
///
/// ```
/// | fn a<T>(items: &[T]) -> Box<dyn Iterator<Item=&T>> {
/// | - let's call the lifetime of this reference `'1`
/// ```
///
/// the way this works is that we match up `argument_ty`, which is
/// a `Ty<'tcx>` (the internal form of the type) with
/// `argument_hir_ty`, a `hir::Ty` (the syntax of the type
/// annotation). We are descending through the types stepwise,
/// looking in to find the region `needle_fr` in the internal
/// type. Once we find that, we can use the span of the `hir::Ty`
/// to add the highlight.
///
/// This is a somewhat imperfect process, so long the way we also
/// keep track of the **closest** type we've found. If we fail to
/// find the exact `&` or `'_` to highlight, then we may fall back
/// to highlighting that closest type instead.
fn give_name_if_we_can_match_hir_ty(
&self,
tcx: TyCtxt<'_, '_, 'tcx>,
needle_fr: RegionVid,
argument_ty: Ty<'tcx>,
argument_hir_ty: &hir::Ty,
counter: &mut usize,
diag: &mut DiagnosticBuilder<'_>,
) -> Option<InternedString> {
let search_stack: &mut Vec<(Ty<'tcx>, &hir::Ty)> = &mut Vec::new();

search_stack.push((argument_ty, argument_hir_ty));

let mut closest_match: &hir::Ty = argument_hir_ty;

while let Some((ty, hir_ty)) = search_stack.pop() {
// While we search, also track the closet match.
if tcx.any_free_region_meets(&ty, |r| r.to_region_vid() == needle_fr) {
closest_match = hir_ty;
}

match (&ty.sty, &hir_ty.node) {
// Check if the `argument_ty` is `&'X ..` where `'X`
// is the region we are looking for -- if so, and we have a `&T`
// on the RHS, then we want to highlight the `&` like so:
//
// &
// - let's call the lifetime of this reference `'1`
(ty::TyRef(region, referent_ty, _), hir::TyRptr(_lifetime, referent_hir_ty)) => {
if region.to_region_vid() == needle_fr {
let region_name = self.synthesize_region_name(counter);

// Just grab the first character, the `&`.
let codemap = tcx.sess.codemap();
let ampersand_span = codemap.start_point(hir_ty.span);

diag.span_label(
ampersand_span,
format!(
"let's call the lifetime of this reference `{}`",
region_name
),
);

return Some(region_name);
}

// Otherwise, let's descend into the referent types.
search_stack.push((referent_ty, &referent_hir_ty.ty));
}

// Match up something like `Foo<'1>`
(ty::TyAdt(_adt_def, substs), hir::TyPath(hir::QPath::Resolved(None, path))) => {
if let Some(last_segment) = path.segments.last() {
if let Some(name) = self.match_adt_and_segment(
substs,
needle_fr,
last_segment,
counter,
diag,
search_stack,
) {
return Some(name);
}
}
}

// The following cases don't have lifetimes, so we
// just worry about trying to match up the rustc type
// with the HIR types:
(ty::TyTuple(elem_tys), hir::TyTup(elem_hir_tys)) => {
search_stack.extend(elem_tys.iter().cloned().zip(elem_hir_tys));
}

(ty::TySlice(elem_ty), hir::TySlice(elem_hir_ty))
| (ty::TyArray(elem_ty, _), hir::TyArray(elem_hir_ty, _)) => {
search_stack.push((elem_ty, elem_hir_ty));
}

(ty::TyRawPtr(mut_ty), hir::TyPtr(mut_hir_ty)) => {
search_stack.push((mut_ty.ty, &mut_hir_ty.ty));
}

_ => {
// FIXME there are other cases that we could trace
}
}
}

let region_name = self.synthesize_region_name(counter);
diag.span_label(
closest_match.span,
format!("lifetime `{}` appears in this type", region_name),
);

return Some(region_name);
}

/// We've found an enum/struct/union type with the substitutions
/// `substs` and -- in the HIR -- a path type with the final
/// segment `last_segment`. Try to find a `'_` to highlight in
/// the generic args (or, if not, to produce new zipped pairs of
/// types+hir to search through).
fn match_adt_and_segment<'hir>(
&self,
substs: &'tcx Substs<'tcx>,
needle_fr: RegionVid,
last_segment: &'hir hir::PathSegment,
counter: &mut usize,
diag: &mut DiagnosticBuilder<'_>,
search_stack: &mut Vec<(Ty<'tcx>, &'hir hir::Ty)>,
) -> Option<InternedString> {
// Did the user give explicit arguments? (e.g., `Foo<..>`)
let args = last_segment.args.as_ref()?;
let lifetime = self.try_match_adt_and_generic_args(substs, needle_fr, args, search_stack)?;
match lifetime.name {
hir::LifetimeName::Param(_)
| hir::LifetimeName::Static
| hir::LifetimeName::Underscore => {
let region_name = self.synthesize_region_name(counter);
let ampersand_span = lifetime.span;
diag.span_label(ampersand_span, format!("let's call this `{}`", region_name));
return Some(region_name);
}

hir::LifetimeName::Implicit => {
// In this case, the user left off the lifetime; so
// they wrote something like:
//
// ```
// x: Foo<T>
// ```
//
// where the fully elaborated form is `Foo<'_, '1,
// T>`. We don't consider this a match; instead we let
// the "fully elaborated" type fallback above handle
// it.
return None;
}
}
}

/// We've found an enum/struct/union type with the substitutions
/// `substs` and -- in the HIR -- a path with the generic
/// arguments `args`. If `needle_fr` appears in the args, return
/// the `hir::Lifetime` that corresponds to it. If not, push onto
/// `search_stack` the types+hir to search through.
fn try_match_adt_and_generic_args<'hir>(
&self,
substs: &'tcx Substs<'tcx>,
needle_fr: RegionVid,
args: &'hir hir::GenericArgs,
search_stack: &mut Vec<(Ty<'tcx>, &'hir hir::Ty)>,
) -> Option<&'hir hir::Lifetime> {
for (kind, hir_arg) in substs.iter().zip(&args.args) {
match (kind.unpack(), hir_arg) {
(UnpackedKind::Lifetime(r), hir::GenericArg::Lifetime(lt)) => {
if r.to_region_vid() == needle_fr {
return Some(lt);
}
}

(UnpackedKind::Type(ty), hir::GenericArg::Type(hir_ty)) => {
search_stack.push((ty, hir_ty));
}

(UnpackedKind::Lifetime(_), _) | (UnpackedKind::Type(_), _) => {
// I *think* that HIR lowering should ensure this
// doesn't happen, even in erroneous
// programs. Else we should use delay-span-bug.
span_bug!(
hir_arg.span(),
"unmatched subst and hir arg: found {:?} vs {:?}",
kind,
hir_arg,
);
}
}
}

None
}

/// Find a closure upvar that contains `fr` and label it with a
/// fully elaborated type, returning something like `'1`. Result
/// looks like:
Expand All @@ -178,7 +434,8 @@ impl<'tcx> RegionInferenceContext<'tcx> {
counter: &mut usize,
diag: &mut DiagnosticBuilder<'_>,
) -> Option<InternedString> {
let upvar_index = self.universal_regions
let upvar_index = self
.universal_regions
.defining_ty
.upvar_tys(tcx)
.position(|upvar_ty| {
Expand All @@ -189,15 +446,16 @@ impl<'tcx> RegionInferenceContext<'tcx> {
tcx.any_free_region_meets(&upvar_ty, |r| r.to_region_vid() == fr)
})?;

let upvar_ty = self
.universal_regions
.defining_ty
.upvar_tys(tcx)
.nth(upvar_index);

debug!(
"give_name_if_anonymous_region_appears_in_upvars: \
found {:?} in upvar {} which has type {:?}",
fr,
upvar_index,
self.universal_regions
.defining_ty
.upvar_tys(tcx)
.nth(upvar_index),
fr, upvar_index, upvar_ty,
);

let region_name = self.synthesize_region_name(counter);
Expand Down Expand Up @@ -229,9 +487,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {
counter: &mut usize,
diag: &mut DiagnosticBuilder<'_>,
) -> Option<InternedString> {
let return_ty = self.universal_regions
.unnormalized_output_ty;
debug!("give_name_if_anonymous_region_appears_in_output: return_ty = {:?}", return_ty);
let return_ty = self.universal_regions.unnormalized_output_ty;
debug!(
"give_name_if_anonymous_region_appears_in_output: return_ty = {:?}",
return_ty
);
if !tcx.any_free_region_meets(&return_ty, |r| r.to_region_vid() == fr) {
return None;
}
Expand Down
9 changes: 9 additions & 0 deletions src/libsyntax/codemap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,15 @@ impl CodeMap {
self.span_until_char(sp, '{')
}

/// Returns a new span representing just the start-point of this span
pub fn start_point(&self, sp: Span) -> Span {
let pos = sp.lo().0;
let width = self.find_width_of_character_at_span(sp, false);
let corrected_start_position = pos.checked_add(width).unwrap_or(pos);
let end_point = BytePos(cmp::max(corrected_start_position, sp.lo().0));
sp.with_hi(end_point)
}

/// Returns a new span representing just the end-point of this span
pub fn end_point(&self, sp: Span) -> Span {
let pos = sp.hi().0;
Expand Down
Loading

0 comments on commit fa9f580

Please sign in to comment.