Skip to content

Commit

Permalink
Rollup merge of rust-lang#120704 - amandasystems:silly-region-name-re…
Browse files Browse the repository at this point in the history
…write, r=compiler-errors

A drive-by rewrite of `give_region_a_name()`

This drive-by rewrite makes the cache-updating nature of the method clearer, using the Entry API into the hash table for region names to capture the update-insert nature of the method. May be marginally more efficient since it only runtime-borrows and indexes the map once, but in this context the performance impact is almost certainly completely negligible.

Note that this commit should preserve all externally visible behaviour. Notably, it preserves the debug logging:

1. printing even in the case of a `None` for the new computed name, and
2. only printing on new values, begin silent on reused values
  • Loading branch information
matthiaskrgr authored Feb 9, 2024
2 parents e9a8a3d + 795be51 commit 4ba943f
Showing 1 changed file with 31 additions and 27 deletions.
58 changes: 31 additions & 27 deletions compiler/rustc_borrowck/src/diagnostics/region_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
use std::fmt::{self, Display};
use std::iter;

use rustc_data_structures::fx::IndexEntry;
use rustc_errors::Diagnostic;
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
Expand All @@ -17,7 +18,7 @@ use crate::{universal_regions::DefiningTy, MirBorrowckCtxt};

/// A name for a particular region used in emitting diagnostics. This name could be a generated
/// name like `'1`, a name used by the user like `'a`, or a name like `'static`.
#[derive(Debug, Clone)]
#[derive(Debug, Clone, Copy)]
pub(crate) struct RegionName {
/// The name of the region (interned).
pub(crate) name: Symbol,
Expand All @@ -28,7 +29,7 @@ pub(crate) struct RegionName {
/// Denotes the source of a region that is named by a `RegionName`. For example, a free region that
/// was named by the user would get `NamedLateParamRegion` and `'static` lifetime would get `Static`.
/// This helps to print the right kinds of diagnostics.
#[derive(Debug, Clone)]
#[derive(Debug, Clone, Copy)]
pub(crate) enum RegionNameSource {
/// A bound (not free) region that was instantiated at the def site (not an HRTB).
NamedEarlyParamRegion(Span),
Expand All @@ -45,7 +46,7 @@ pub(crate) enum RegionNameSource {
/// The region corresponding to the return type of a closure.
AnonRegionFromOutput(RegionNameHighlight, &'static str),
/// The region from a type yielded by a coroutine.
AnonRegionFromYieldTy(Span, String),
AnonRegionFromYieldTy(Span, Symbol),
/// An anonymous region from an async fn.
AnonRegionFromAsyncFn(Span),
/// An anonymous region from an impl self type or trait
Expand All @@ -54,19 +55,19 @@ pub(crate) enum RegionNameSource {

/// Describes what to highlight to explain to the user that we're giving an anonymous region a
/// synthesized name, and how to highlight it.
#[derive(Debug, Clone)]
#[derive(Debug, Clone, Copy)]
pub(crate) enum RegionNameHighlight {
/// The anonymous region corresponds to a reference that was found by traversing the type in the HIR.
MatchedHirTy(Span),
/// The anonymous region corresponds to a `'_` in the generics list of a struct/enum/union.
MatchedAdtAndSegment(Span),
/// The anonymous region corresponds to a region where the type annotation is completely missing
/// from the code, e.g. in a closure arguments `|x| { ... }`, where `x` is a reference.
CannotMatchHirTy(Span, String),
CannotMatchHirTy(Span, Symbol),
/// The anonymous region corresponds to a region where the type annotation is completely missing
/// from the code, and *even if* we print out the full name of the type, the region name won't
/// be included. This currently occurs for opaque types like `impl Future`.
Occluded(Span, String),
Occluded(Span, Symbol),
}

impl RegionName {
Expand Down Expand Up @@ -250,25 +251,28 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> {

assert!(self.regioncx.universal_regions().is_universal_region(fr));

if let Some(value) = self.region_names.try_borrow_mut().unwrap().get(&fr) {
return Some(value.clone());
}
match self.region_names.borrow_mut().entry(fr) {
IndexEntry::Occupied(precomputed_name) => Some(*precomputed_name.get()),
IndexEntry::Vacant(slot) => {
let new_name = self
.give_name_from_error_region(fr)
.or_else(|| self.give_name_if_anonymous_region_appears_in_arguments(fr))
.or_else(|| self.give_name_if_anonymous_region_appears_in_upvars(fr))
.or_else(|| self.give_name_if_anonymous_region_appears_in_output(fr))
.or_else(|| self.give_name_if_anonymous_region_appears_in_yield_ty(fr))
.or_else(|| self.give_name_if_anonymous_region_appears_in_impl_signature(fr))
.or_else(|| {
self.give_name_if_anonymous_region_appears_in_arg_position_impl_trait(fr)
});

if let Some(new_name) = new_name {
slot.insert(new_name);
}
debug!("give_region_a_name: gave name {:?}", new_name);

let value = self
.give_name_from_error_region(fr)
.or_else(|| self.give_name_if_anonymous_region_appears_in_arguments(fr))
.or_else(|| self.give_name_if_anonymous_region_appears_in_upvars(fr))
.or_else(|| self.give_name_if_anonymous_region_appears_in_output(fr))
.or_else(|| self.give_name_if_anonymous_region_appears_in_yield_ty(fr))
.or_else(|| self.give_name_if_anonymous_region_appears_in_impl_signature(fr))
.or_else(|| self.give_name_if_anonymous_region_appears_in_arg_position_impl_trait(fr));

if let Some(value) = &value {
self.region_names.try_borrow_mut().unwrap().insert(fr, value.clone());
new_name
}
}

debug!("give_region_a_name: gave name {:?}", value);
value
}

/// Checks for the case where `fr` maps to something that the
Expand Down Expand Up @@ -460,9 +464,9 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> {
);
if type_name.contains(&format!("'{counter}")) {
// Only add a label if we can confirm that a region was labelled.
RegionNameHighlight::CannotMatchHirTy(span, type_name)
RegionNameHighlight::CannotMatchHirTy(span, Symbol::intern(&type_name))
} else {
RegionNameHighlight::Occluded(span, type_name)
RegionNameHighlight::Occluded(span, Symbol::intern(&type_name))
}
}

Expand Down Expand Up @@ -891,7 +895,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> {

Some(RegionName {
name: self.synthesize_region_name(),
source: RegionNameSource::AnonRegionFromYieldTy(yield_span, type_name),
source: RegionNameSource::AnonRegionFromYieldTy(yield_span, Symbol::intern(&type_name)),
})
}

Expand Down Expand Up @@ -983,7 +987,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> {
Some(RegionName {
name: region_name,
source: RegionNameSource::AnonRegionFromArgument(
RegionNameHighlight::CannotMatchHirTy(arg_span, arg_name?.to_string()),
RegionNameHighlight::CannotMatchHirTy(arg_span, arg_name?),
),
})
} else {
Expand Down

0 comments on commit 4ba943f

Please sign in to comment.