From ad3d04c55ec4cbd924b3f7e3a890483b6eb1604d Mon Sep 17 00:00:00 2001
From: Amanda Stjerna <amanda.stjerna@it.uu.se>
Date: Tue, 6 Feb 2024 13:07:51 +0100
Subject: [PATCH 1/2] A drive-by rewrite of give_region_a_name()
This rewrite makes the cache-updating nature of the function slightly 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 the map once, but in this context the performance impact is almost certainly completely negligible.
---
.../src/diagnostics/region_name.rs | 38 ++++++++++---------
1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/compiler/rustc_borrowck/src/diagnostics/region_name.rs b/compiler/rustc_borrowck/src/diagnostics/region_name.rs
index 15e1066e983a9..c9de36e9064e2 100644
--- a/compiler/rustc_borrowck/src/diagnostics/region_name.rs
+++ b/compiler/rustc_borrowck/src/diagnostics/region_name.rs
@@ -1,6 +1,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};
@@ -247,25 +248,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().clone()),
+ 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.clone());
+ }
+ 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
From 795be51dd931848b8cbf3dc84c526ca92634803c Mon Sep 17 00:00:00 2001
From: Amanda Stjerna <amanda.stjerna@it.uu.se>
Date: Wed, 7 Feb 2024 14:51:51 +0100
Subject: [PATCH 2/2] Make `RegionName` `Copy` by (transitively) interning the
few string variants
---
.../src/diagnostics/region_name.rs | 26 +++++++++----------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/compiler/rustc_borrowck/src/diagnostics/region_name.rs b/compiler/rustc_borrowck/src/diagnostics/region_name.rs
index c9de36e9064e2..a31e6ce5f297f 100644
--- a/compiler/rustc_borrowck/src/diagnostics/region_name.rs
+++ b/compiler/rustc_borrowck/src/diagnostics/region_name.rs
@@ -15,7 +15,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,
@@ -26,7 +26,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),
@@ -43,7 +43,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
@@ -52,7 +52,7 @@ 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),
@@ -60,11 +60,11 @@ pub(crate) enum RegionNameHighlight {
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 {
@@ -249,7 +249,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> {
assert!(self.regioncx.universal_regions().is_universal_region(fr));
match self.region_names.borrow_mut().entry(fr) {
- IndexEntry::Occupied(precomputed_name) => Some(precomputed_name.get().clone()),
+ IndexEntry::Occupied(precomputed_name) => Some(*precomputed_name.get()),
IndexEntry::Vacant(slot) => {
let new_name = self
.give_name_from_error_region(fr)
@@ -262,8 +262,8 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> {
self.give_name_if_anonymous_region_appears_in_arg_position_impl_trait(fr)
});
- if let Some(new_name) = &new_name {
- slot.insert(new_name.clone());
+ if let Some(new_name) = new_name {
+ slot.insert(new_name);
}
debug!("give_region_a_name: gave name {:?}", new_name);
@@ -460,9 +460,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))
}
}
@@ -882,7 +882,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)),
})
}
@@ -974,7 +974,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 {