1
Fork 0

Refactor region naming for control of diagnostics.

Previously, region naming would always highlight the source of the
region name it found. Now, region naming returns the name as part
of a larger structure that encodes the source of the region naming
such that a region name can be optionally added to the diagnostic.
This commit is contained in:
David Wood 2018-09-11 13:27:39 +02:00
parent 10af6a2b37
commit 9e3889e2ea
No known key found for this signature in database
GPG key ID: 01760B4F9F53F154
2 changed files with 168 additions and 125 deletions

View file

@ -339,10 +339,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {
); );
let counter = &mut 1; let counter = &mut 1;
let fr_name = self.give_region_a_name( let fr_name = self.give_region_a_name(infcx, mir, mir_def_id, fr, counter);
infcx, mir, mir_def_id, fr, counter, &mut diag); fr_name.highlight_region_name(&mut diag);
let outlived_fr_name = self.give_region_a_name( let outlived_fr_name = self.give_region_a_name(
infcx, mir, mir_def_id, outlived_fr, counter, &mut diag); infcx, mir, mir_def_id, outlived_fr, counter);
outlived_fr_name.highlight_region_name(&mut diag);
let mir_def_name = if infcx.tcx.is_closure(mir_def_id) { "closure" } else { "function" }; let mir_def_name = if infcx.tcx.is_closure(mir_def_id) { "closure" } else { "function" };
@ -430,10 +431,12 @@ impl<'tcx> RegionInferenceContext<'tcx> {
// Otherwise, we should suggest adding a constraint on the return type. // Otherwise, we should suggest adding a constraint on the return type.
let span = infcx.tcx.def_span(*did); let span = infcx.tcx.def_span(*did);
if let Ok(snippet) = infcx.tcx.sess.source_map().span_to_snippet(span) { if let Ok(snippet) = infcx.tcx.sess.source_map().span_to_snippet(span) {
let suggestable_fr_name = match fr_name { let suggestable_fr_name = if fr_name.was_named() {
RegionName::Named(name) => format!("{}", name), format!("{}", fr_name)
RegionName::Synthesized(_) => "'_".to_string(), } else {
"'_".to_string()
}; };
diag.span_suggestion_with_applicability( diag.span_suggestion_with_applicability(
span, span,
&format!( &format!(

View file

@ -25,29 +25,102 @@ use syntax::symbol::keywords;
use syntax_pos::Span; use syntax_pos::Span;
use syntax_pos::symbol::InternedString; use syntax_pos::symbol::InternedString;
/// Name of a region used in error reporting. Variants denote the source of the region name -
/// whether it was synthesized for the error message and therefore should not be used in
/// suggestions; or whether it was found from the region.
#[derive(Debug)] #[derive(Debug)]
pub(crate) enum RegionName { crate struct RegionName {
Named(InternedString), name: InternedString,
Synthesized(InternedString), source: RegionNameSource,
}
#[derive(Debug)]
crate enum RegionNameSource {
NamedEarlyBoundRegion(Span),
NamedFreeRegion(Span),
Static,
SynthesizedFreeEnvRegion(Span, String),
CannotMatchHirTy(Span, String),
MatchedHirTy(Span),
MatchedAdtAndSegment(Span),
AnonRegionFromUpvar(Span, String),
AnonRegionFromOutput(Span, String, String),
} }
impl RegionName { impl RegionName {
fn as_interned_string(&self) -> &InternedString { #[allow(dead_code)]
match self { crate fn was_named(&self) -> bool {
RegionName::Named(name) | RegionName::Synthesized(name) => name, match self.source {
RegionNameSource::NamedEarlyBoundRegion(..) |
RegionNameSource::NamedFreeRegion(..) |
RegionNameSource::Static => true,
RegionNameSource::SynthesizedFreeEnvRegion(..) |
RegionNameSource::CannotMatchHirTy(..) |
RegionNameSource::MatchedHirTy(..) |
RegionNameSource::MatchedAdtAndSegment(..) |
RegionNameSource::AnonRegionFromUpvar(..) |
RegionNameSource::AnonRegionFromOutput(..) => false,
}
}
#[allow(dead_code)]
crate fn was_synthesized(&self) -> bool {
!self.was_named()
}
#[allow(dead_code)]
crate fn name(&self) -> &InternedString {
&self.name
}
crate fn highlight_region_name(
&self,
diag: &mut DiagnosticBuilder<'_>
) {
match &self.source {
RegionNameSource::NamedFreeRegion(span) |
RegionNameSource::NamedEarlyBoundRegion(span) => {
diag.span_label(
*span,
format!("lifetime `{}` defined here", self),
);
},
RegionNameSource::SynthesizedFreeEnvRegion(span, note) => {
diag.span_label(
*span,
format!("lifetime `{}` represents this closure's body", self),
);
diag.note(&note);
},
RegionNameSource::CannotMatchHirTy(span, type_name) => {
diag.span_label(*span, format!("has type `{}`", type_name));
},
RegionNameSource::MatchedHirTy(span) => {
diag.span_label(
*span,
format!("let's call the lifetime of this reference `{}`", self),
);
},
RegionNameSource::MatchedAdtAndSegment(span) => {
diag.span_label(*span, format!("let's call this `{}`", self));
},
RegionNameSource::AnonRegionFromUpvar(span, upvar_name) => {
diag.span_label(
*span,
format!("lifetime `{}` appears in the type of `{}`", self, upvar_name),
);
},
RegionNameSource::AnonRegionFromOutput(span, mir_description, type_name) => {
diag.span_label(
*span,
format!("return type{} is {}", mir_description, type_name),
);
},
RegionNameSource::Static => {},
} }
} }
} }
impl Display for RegionName { impl Display for RegionName {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self { write!(f, "{}", self.name)
RegionName::Named(name) | RegionName::Synthesized(name) =>
write!(f, "{}", name),
}
} }
} }
@ -84,26 +157,25 @@ impl<'tcx> RegionInferenceContext<'tcx> {
mir_def_id: DefId, mir_def_id: DefId,
fr: RegionVid, fr: RegionVid,
counter: &mut usize, counter: &mut usize,
diag: &mut DiagnosticBuilder,
) -> RegionName { ) -> RegionName {
debug!("give_region_a_name(fr={:?}, counter={})", fr, counter); debug!("give_region_a_name(fr={:?}, counter={})", fr, counter);
assert!(self.universal_regions.is_universal_region(fr)); assert!(self.universal_regions.is_universal_region(fr));
let value = self.give_name_from_error_region(infcx.tcx, mir_def_id, fr, counter, diag) let value = self.give_name_from_error_region(infcx.tcx, mir_def_id, fr, counter)
.or_else(|| { .or_else(|| {
self.give_name_if_anonymous_region_appears_in_arguments( self.give_name_if_anonymous_region_appears_in_arguments(
infcx, mir, mir_def_id, fr, counter, diag, infcx, mir, mir_def_id, fr, counter,
) )
}) })
.or_else(|| { .or_else(|| {
self.give_name_if_anonymous_region_appears_in_upvars( self.give_name_if_anonymous_region_appears_in_upvars(
infcx.tcx, mir, fr, counter, diag, infcx.tcx, mir, fr, counter,
) )
}) })
.or_else(|| { .or_else(|| {
self.give_name_if_anonymous_region_appears_in_output( self.give_name_if_anonymous_region_appears_in_output(
infcx, mir, mir_def_id, fr, counter, diag, infcx, mir, mir_def_id, fr, counter,
) )
}) })
.unwrap_or_else(|| span_bug!(mir.span, "can't make a name for free region {:?}", fr)); .unwrap_or_else(|| span_bug!(mir.span, "can't make a name for free region {:?}", fr));
@ -122,7 +194,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
mir_def_id: DefId, mir_def_id: DefId,
fr: RegionVid, fr: RegionVid,
counter: &mut usize, counter: &mut usize,
diag: &mut DiagnosticBuilder<'_>,
) -> Option<RegionName> { ) -> Option<RegionName> {
let error_region = self.to_error_region(fr)?; let error_region = self.to_error_region(fr)?;
@ -130,23 +201,28 @@ impl<'tcx> RegionInferenceContext<'tcx> {
match error_region { match error_region {
ty::ReEarlyBound(ebr) => { ty::ReEarlyBound(ebr) => {
if ebr.has_name() { if ebr.has_name() {
let name = RegionName::Named(ebr.name); let span = self.get_named_span(tcx, error_region, &ebr.name);
self.highlight_named_span(tcx, error_region, &name, diag); Some(RegionName {
Some(name) name: ebr.name,
source: RegionNameSource::NamedEarlyBoundRegion(span)
})
} else { } else {
None None
} }
} }
ty::ReStatic => Some(RegionName::Named( ty::ReStatic => Some(RegionName {
keywords::StaticLifetime.name().as_interned_str() name: keywords::StaticLifetime.name().as_interned_str(),
)), source: RegionNameSource::Static
}),
ty::ReFree(free_region) => match free_region.bound_region { ty::ReFree(free_region) => match free_region.bound_region {
ty::BoundRegion::BrNamed(_, name) => { ty::BoundRegion::BrNamed(_, name) => {
let name = RegionName::Named(name); let span = self.get_named_span(tcx, error_region, &name);
self.highlight_named_span(tcx, error_region, &name, diag); Some(RegionName {
Some(name) name,
source: RegionNameSource::NamedFreeRegion(span),
})
}, },
ty::BoundRegion::BrEnv => { ty::BoundRegion::BrEnv => {
@ -162,13 +238,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
bug!("Closure is not defined by a closure expr"); bug!("Closure is not defined by a closure expr");
}; };
let region_name = self.synthesize_region_name(counter); let region_name = self.synthesize_region_name(counter);
diag.span_label(
args_span,
format!(
"lifetime `{}` represents this closure's body",
region_name
),
);
let closure_kind_ty = substs.closure_kind_ty(def_id, tcx); let closure_kind_ty = substs.closure_kind_ty(def_id, tcx);
let note = match closure_kind_ty.to_opt_closure_kind() { let note = match closure_kind_ty.to_opt_closure_kind() {
@ -186,9 +255,13 @@ impl<'tcx> RegionInferenceContext<'tcx> {
None => bug!("Closure kind not inferred in borrow check"), None => bug!("Closure kind not inferred in borrow check"),
}; };
diag.note(note); Some(RegionName {
name: region_name,
Some(region_name) source: RegionNameSource::SynthesizedFreeEnvRegion(
args_span,
note.to_string()
),
})
} else { } else {
// Can't have BrEnv in functions, constants or generators. // Can't have BrEnv in functions, constants or generators.
bug!("BrEnv outside of closure."); bug!("BrEnv outside of closure.");
@ -209,27 +282,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
} }
} }
/// Get the span of a named region. /// Get a span of a named region to provide context for error messages that
pub(super) fn get_span_of_named_region(
&self,
tcx: TyCtxt<'_, '_, 'tcx>,
error_region: &RegionKind,
name: &RegionName,
) -> Span {
let scope = error_region.free_region_binding_scope(tcx);
let node = tcx.hir.as_local_node_id(scope).unwrap_or(DUMMY_NODE_ID);
let span = tcx.sess.source_map().def_span(tcx.hir.span(node));
if let Some(param) = tcx.hir.get_generics(scope).and_then(|generics| {
generics.get_named(name.as_interned_string())
}) {
param.span
} else {
span
}
}
/// Highlight a named span to provide context for error messages that
/// mention that span, for example: /// mention that span, for example:
/// ///
/// ``` /// ```
@ -243,19 +296,24 @@ impl<'tcx> RegionInferenceContext<'tcx> {
/// | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ argument requires that `'b` must /// | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ argument requires that `'b` must
/// | outlive `'a` /// | outlive `'a`
/// ``` /// ```
fn highlight_named_span( fn get_named_span(
&self, &self,
tcx: TyCtxt<'_, '_, 'tcx>, tcx: TyCtxt<'_, '_, 'tcx>,
error_region: &RegionKind, error_region: &RegionKind,
name: &RegionName, name: &InternedString,
diag: &mut DiagnosticBuilder<'_>, ) -> Span {
) { let scope = error_region.free_region_binding_scope(tcx);
let span = self.get_span_of_named_region(tcx, error_region, name); let node = tcx.hir.as_local_node_id(scope).unwrap_or(DUMMY_NODE_ID);
diag.span_label( let span = tcx.sess.source_map().def_span(tcx.hir.span(node));
span, if let Some(param) = tcx.hir
format!("lifetime `{}` defined here", name), .get_generics(scope)
); .and_then(|generics| generics.get_named(name))
{
param.span
} else {
span
}
} }
/// Find an argument that contains `fr` and label it with a fully /// Find an argument that contains `fr` and label it with a fully
@ -273,7 +331,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
mir_def_id: DefId, mir_def_id: DefId,
fr: RegionVid, fr: RegionVid,
counter: &mut usize, counter: &mut usize,
diag: &mut DiagnosticBuilder<'_>,
) -> Option<RegionName> { ) -> Option<RegionName> {
let implicit_inputs = self.universal_regions.defining_ty.implicit_inputs(); let implicit_inputs = self.universal_regions.defining_ty.implicit_inputs();
let argument_index = self.get_argument_index_for_region(infcx.tcx, fr)?; let argument_index = self.get_argument_index_for_region(infcx.tcx, fr)?;
@ -288,12 +345,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {
arg_ty, arg_ty,
argument_index, argument_index,
counter, counter,
diag,
) { ) {
return Some(region_name); return Some(region_name);
} }
self.give_name_if_we_cannot_match_hir_ty(infcx, mir, fr, arg_ty, counter, diag) self.give_name_if_we_cannot_match_hir_ty(infcx, mir, fr, arg_ty, counter)
} }
fn give_name_if_we_can_match_hir_ty_from_argument( fn give_name_if_we_can_match_hir_ty_from_argument(
@ -305,7 +361,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
argument_ty: Ty<'tcx>, argument_ty: Ty<'tcx>,
argument_index: usize, argument_index: usize,
counter: &mut usize, counter: &mut usize,
diag: &mut DiagnosticBuilder<'_>,
) -> Option<RegionName> { ) -> Option<RegionName> {
let mir_node_id = infcx.tcx.hir.as_local_node_id(mir_def_id)?; let mir_node_id = infcx.tcx.hir.as_local_node_id(mir_def_id)?;
let fn_decl = infcx.tcx.hir.fn_decl(mir_node_id)?; let fn_decl = infcx.tcx.hir.fn_decl(mir_node_id)?;
@ -320,7 +375,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
needle_fr, needle_fr,
argument_ty, argument_ty,
counter, counter,
diag,
), ),
_ => self.give_name_if_we_can_match_hir_ty( _ => self.give_name_if_we_can_match_hir_ty(
@ -329,7 +383,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
argument_ty, argument_ty,
argument_hir_ty, argument_hir_ty,
counter, counter,
diag,
), ),
} }
} }
@ -352,7 +405,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
needle_fr: RegionVid, needle_fr: RegionVid,
argument_ty: Ty<'tcx>, argument_ty: Ty<'tcx>,
counter: &mut usize, counter: &mut usize,
diag: &mut DiagnosticBuilder<'_>,
) -> Option<RegionName> { ) -> Option<RegionName> {
let type_name = with_highlight_region(needle_fr, *counter, || { let type_name = with_highlight_region(needle_fr, *counter, || {
infcx.extract_type_name(&argument_ty) infcx.extract_type_name(&argument_ty)
@ -366,12 +418,14 @@ impl<'tcx> RegionInferenceContext<'tcx> {
// Only add a label if we can confirm that a region was labelled. // Only add a label if we can confirm that a region was labelled.
let argument_index = self.get_argument_index_for_region(infcx.tcx, needle_fr)?; let argument_index = self.get_argument_index_for_region(infcx.tcx, needle_fr)?;
let (_, span) = self.get_argument_name_and_span_for_region(mir, argument_index); let (_, span) = self.get_argument_name_and_span_for_region(mir, argument_index);
diag.span_label(span, format!("has type `{}`", type_name));
// This counter value will already have been used, so this function will increment it Some(RegionName {
// so the next value will be used next and return the region name that would have been // This counter value will already have been used, so this function will increment it
// used. // so the next value will be used next and return the region name that would have been
Some(self.synthesize_region_name(counter)) // used.
name: self.synthesize_region_name(counter),
source: RegionNameSource::CannotMatchHirTy(span, type_name),
})
} else { } else {
None None
}; };
@ -407,7 +461,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
argument_ty: Ty<'tcx>, argument_ty: Ty<'tcx>,
argument_hir_ty: &hir::Ty, argument_hir_ty: &hir::Ty,
counter: &mut usize, counter: &mut usize,
diag: &mut DiagnosticBuilder<'_>,
) -> Option<RegionName> { ) -> Option<RegionName> {
let search_stack: &mut Vec<(Ty<'tcx>, &hir::Ty)> = &mut Vec::new(); let search_stack: &mut Vec<(Ty<'tcx>, &hir::Ty)> = &mut Vec::new();
@ -432,15 +485,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
let source_map = tcx.sess.source_map(); let source_map = tcx.sess.source_map();
let ampersand_span = source_map.start_point(hir_ty.span); let ampersand_span = source_map.start_point(hir_ty.span);
diag.span_label( return Some(RegionName {
ampersand_span, name: region_name,
format!( source: RegionNameSource::MatchedHirTy(ampersand_span),
"let's call the lifetime of this reference `{}`", });
region_name
),
);
return Some(region_name);
} }
// Otherwise, let's descend into the referent types. // Otherwise, let's descend into the referent types.
@ -464,7 +512,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
needle_fr, needle_fr,
last_segment, last_segment,
counter, counter,
diag,
search_stack, search_stack,
) { ) {
return Some(name); return Some(name);
@ -509,7 +556,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
needle_fr: RegionVid, needle_fr: RegionVid,
last_segment: &'hir hir::PathSegment, last_segment: &'hir hir::PathSegment,
counter: &mut usize, counter: &mut usize,
diag: &mut DiagnosticBuilder<'_>,
search_stack: &mut Vec<(Ty<'tcx>, &'hir hir::Ty)>, search_stack: &mut Vec<(Ty<'tcx>, &'hir hir::Ty)>,
) -> Option<RegionName> { ) -> Option<RegionName> {
// Did the user give explicit arguments? (e.g., `Foo<..>`) // Did the user give explicit arguments? (e.g., `Foo<..>`)
@ -521,11 +567,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
| hir::LifetimeName::Underscore => { | hir::LifetimeName::Underscore => {
let region_name = self.synthesize_region_name(counter); let region_name = self.synthesize_region_name(counter);
let ampersand_span = lifetime.span; let ampersand_span = lifetime.span;
diag.span_label( return Some(RegionName {
ampersand_span, name: region_name,
format!("let's call this `{}`", region_name) source: RegionNameSource::MatchedAdtAndSegment(ampersand_span),
); });
return Some(region_name);
} }
hir::LifetimeName::Implicit => { hir::LifetimeName::Implicit => {
@ -600,22 +645,16 @@ impl<'tcx> RegionInferenceContext<'tcx> {
mir: &Mir<'tcx>, mir: &Mir<'tcx>,
fr: RegionVid, fr: RegionVid,
counter: &mut usize, counter: &mut usize,
diag: &mut DiagnosticBuilder<'_>,
) -> Option<RegionName> { ) -> Option<RegionName> {
let upvar_index = self.get_upvar_index_for_region(tcx, fr)?; let upvar_index = self.get_upvar_index_for_region(tcx, fr)?;
let (upvar_name, upvar_span) = let (upvar_name, upvar_span) =
self.get_upvar_name_and_span_for_region(tcx, mir, upvar_index); self.get_upvar_name_and_span_for_region(tcx, mir, upvar_index);
let region_name = self.synthesize_region_name(counter); let region_name = self.synthesize_region_name(counter);
diag.span_label( Some(RegionName {
upvar_span, name: region_name,
format!( source: RegionNameSource::AnonRegionFromUpvar(upvar_span, upvar_name.to_string()),
"lifetime `{}` appears in the type of `{}`", })
region_name, upvar_name
),
);
Some(region_name)
} }
/// Check for arguments appearing in the (closure) return type. It /// Check for arguments appearing in the (closure) return type. It
@ -629,7 +668,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
mir_def_id: DefId, mir_def_id: DefId,
fr: RegionVid, fr: RegionVid,
counter: &mut usize, counter: &mut usize,
diag: &mut DiagnosticBuilder<'_>,
) -> Option<RegionName> { ) -> Option<RegionName> {
let tcx = infcx.tcx; let tcx = infcx.tcx;
@ -666,23 +704,25 @@ impl<'tcx> RegionInferenceContext<'tcx> {
(mir.span, "") (mir.span, "")
}; };
diag.span_label( Some(RegionName {
return_span, // This counter value will already have been used, so this function will increment it
format!("return type{} is {}", mir_description, type_name), // so the next value will be used next and return the region name that would have been
); // used.
name: self.synthesize_region_name(counter),
// This counter value will already have been used, so this function will increment it source: RegionNameSource::AnonRegionFromOutput(
// so the next value will be used next and return the region name that would have been return_span,
// used. mir_description.to_string(),
Some(self.synthesize_region_name(counter)) type_name
),
})
} }
/// Create a synthetic region named `'1`, incrementing the /// Create a synthetic region named `'1`, incrementing the
/// counter. /// counter.
fn synthesize_region_name(&self, counter: &mut usize) -> RegionName { fn synthesize_region_name(&self, counter: &mut usize) -> InternedString {
let c = *counter; let c = *counter;
*counter += 1; *counter += 1;
RegionName::Synthesized(Name::intern(&format!("'{:?}", c)).as_interned_str()) Name::intern(&format!("'{:?}", c)).as_interned_str()
} }
} }