From 4b46271841d75ee8b45b7b4db2ce531a764785fa Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Thu, 12 Mar 2020 19:14:13 -0500 Subject: [PATCH 1/4] Remove a couple of Rc's from RegionInferenceContext --- src/librustc_mir/borrow_check/mod.rs | 28 +++++++++++++++---- .../borrow_check/region_infer/mod.rs | 9 +++--- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index a61d00b0120..8dc6ee7ea3b 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -73,6 +73,24 @@ crate use place_ext::PlaceExt; crate use places_conflict::{places_conflict, PlaceConflictBias}; crate use region_infer::RegionInferenceContext; +/// An owned immutable value. +#[derive(Debug)] +struct Frozen(T); + +impl Frozen { + pub fn freeze(val: T) -> Self { + Frozen(val) + } +} + +impl std::ops::Deref for Frozen { + type Target = T; + + fn deref(&self) -> &T { + &self.0 + } +} + // FIXME(eddyb) perhaps move this somewhere more centrally. #[derive(Debug)] crate struct Upvar { @@ -1577,11 +1595,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { mpi, ); } // Only query longest prefix with a MovePath, not further - // ancestors; dataflow recurs on children when parents - // move (to support partial (re)inits). - // - // (I.e., querying parents breaks scenario 7; but may want - // to do such a query based on partial-init feature-gate.) + // ancestors; dataflow recurs on children when parents + // move (to support partial (re)inits). + // + // (I.e., querying parents breaks scenario 7; but may want + // to do such a query based on partial-init feature-gate.) } /// Subslices correspond to multiple move paths, so we iterate through the diff --git a/src/librustc_mir/borrow_check/region_infer/mod.rs b/src/librustc_mir/borrow_check/region_infer/mod.rs index 144f655420b..43744d78219 100644 --- a/src/librustc_mir/borrow_check/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/region_infer/mod.rs @@ -31,6 +31,7 @@ use crate::borrow_check::{ }, type_check::{free_region_relations::UniversalRegionRelations, Locations}, universal_regions::UniversalRegions, + Frozen, }; mod dump_mir; @@ -54,12 +55,12 @@ pub struct RegionInferenceContext<'tcx> { liveness_constraints: LivenessValues, /// The outlives constraints computed by the type-check. - constraints: Rc, + constraints: Frozen, /// The constraint-set, but in graph form, making it easy to traverse /// the constraints adjacent to a particular region. Used to construct /// the SCC (see `constraint_sccs`) and for error reporting. - constraint_graph: Rc, + constraint_graph: Frozen, /// The SCC computed from `constraints` and the constraint /// graph. We have an edge from SCC A to SCC B if `A: B`. Used to @@ -263,8 +264,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { .map(|info| RegionDefinition::new(info.universe, info.origin)) .collect(); - let constraints = Rc::new(outlives_constraints); // freeze constraints - let constraint_graph = Rc::new(constraints.graph(definitions.len())); + let constraints = Frozen::freeze(outlives_constraints); + let constraint_graph = Frozen::freeze(constraints.graph(definitions.len())); let fr_static = universal_regions.fr_static; let constraint_sccs = Rc::new(constraints.compute_sccs(&constraint_graph, fr_static)); From 508d4a24d1f4ba79bfa01fd6af919609ebeaccbe Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Thu, 12 Mar 2020 19:29:54 -0500 Subject: [PATCH 2/4] Remove another Rc from RegionInferenceContext --- src/librustc_mir/borrow_check/region_infer/mod.rs | 6 +++--- .../borrow_check/type_check/free_region_relations.rs | 5 +++-- src/librustc_mir/borrow_check/type_check/mod.rs | 3 ++- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/librustc_mir/borrow_check/region_infer/mod.rs b/src/librustc_mir/borrow_check/region_infer/mod.rs index 43744d78219..089a9b7328e 100644 --- a/src/librustc_mir/borrow_check/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/region_infer/mod.rs @@ -113,7 +113,7 @@ pub struct RegionInferenceContext<'tcx> { /// Information about how the universally quantified regions in /// scope on this function relate to one another. - universal_region_relations: Rc>, + universal_region_relations: Frozen>, } /// Each time that `apply_member_constraint` is successful, it appends @@ -243,11 +243,11 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// /// The `outlives_constraints` and `type_tests` are an initial set /// of constraints produced by the MIR type check. - pub(crate) fn new( + pub(in crate::borrow_check) fn new( var_infos: VarInfos, universal_regions: Rc>, placeholder_indices: Rc, - universal_region_relations: Rc>, + universal_region_relations: Frozen>, outlives_constraints: OutlivesConstraintSet, member_constraints_in: MemberConstraintSet<'tcx, RegionVid>, closure_bounds_mapping: FxHashMap< diff --git a/src/librustc_mir/borrow_check/type_check/free_region_relations.rs b/src/librustc_mir/borrow_check/type_check/free_region_relations.rs index 137216531a3..a18004ce191 100644 --- a/src/librustc_mir/borrow_check/type_check/free_region_relations.rs +++ b/src/librustc_mir/borrow_check/type_check/free_region_relations.rs @@ -15,6 +15,7 @@ use crate::borrow_check::{ type_check::constraint_conversion, type_check::{Locations, MirTypeckRegionConstraints}, universal_regions::UniversalRegions, + Frozen, }; #[derive(Debug)] @@ -52,7 +53,7 @@ type RegionBoundPairs<'tcx> = Vec<(ty::Region<'tcx>, GenericKind<'tcx>)>; type NormalizedInputsAndOutput<'tcx> = Vec>; crate struct CreateResult<'tcx> { - crate universal_region_relations: Rc>, + pub(in crate::borrow_check) universal_region_relations: Frozen>, crate region_bound_pairs: RegionBoundPairs<'tcx>, crate normalized_inputs_and_output: NormalizedInputsAndOutput<'tcx>, } @@ -297,7 +298,7 @@ impl UniversalRegionRelationsBuilder<'cx, 'tcx> { } CreateResult { - universal_region_relations: Rc::new(self.relations), + universal_region_relations: Frozen::freeze(self.relations), region_bound_pairs: self.region_bound_pairs, normalized_inputs_and_output, } diff --git a/src/librustc_mir/borrow_check/type_check/mod.rs b/src/librustc_mir/borrow_check/type_check/mod.rs index ace92949814..f4656673fec 100644 --- a/src/librustc_mir/borrow_check/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/type_check/mod.rs @@ -55,6 +55,7 @@ use crate::borrow_check::{ renumber, type_check::free_region_relations::{CreateResult, UniversalRegionRelations}, universal_regions::{DefiningTy, UniversalRegions}, + Frozen, }; macro_rules! span_mirbug { @@ -828,7 +829,7 @@ struct BorrowCheckContext<'a, 'tcx> { crate struct MirTypeckResults<'tcx> { crate constraints: MirTypeckRegionConstraints<'tcx>, - crate universal_region_relations: Rc>, + pub(in crate::borrow_check) universal_region_relations: Frozen>, crate opaque_type_values: FxHashMap>, } From da4e33a9e659071ae5e7418242dea38d951a260d Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Fri, 13 Mar 2020 13:28:25 -0500 Subject: [PATCH 3/4] move frozen to rustc_data_structures --- src/librustc_data_structures/frozen.rs | 57 +++++++++++++++++++ src/librustc_data_structures/lib.rs | 1 + src/librustc_mir/borrow_check/mod.rs | 28 ++------- .../borrow_check/region_infer/mod.rs | 2 +- .../type_check/free_region_relations.rs | 2 +- .../borrow_check/type_check/mod.rs | 2 +- 6 files changed, 66 insertions(+), 26 deletions(-) create mode 100644 src/librustc_data_structures/frozen.rs diff --git a/src/librustc_data_structures/frozen.rs b/src/librustc_data_structures/frozen.rs new file mode 100644 index 00000000000..835fa7d839c --- /dev/null +++ b/src/librustc_data_structures/frozen.rs @@ -0,0 +1,57 @@ +//! An immutable, owned value. +//! +//! The purpose of `Frozen` is to make a value immutable for the sake of defensive programming. For example, +//! suppose we have the following: +//! +//! ```rust +//! struct Bar { /* some data */ } +//! +//! struct Foo { +//! /// Some computed data that should never change after construction. +//! pub computed: Bar, +//! +//! /* some other fields */ +//! } +//! +//! impl Bar { +//! /// Mutate the `Bar`. +//! pub fn mutate(&mut self) { } +//! } +//! ``` +//! +//! Now suppose we want to pass around a mutable `Foo` instance but, we want to make sure that +//! `computed` does not change accidentally (e.g. somebody might accidentally call +//! `foo.computed.mutate()`). This is what `Frozen` is for. We can do the following: +//! +//! ```rust +//! use rustc_data_structures::frozen::Frozen; +//! +//! struct Foo { +//! /// Some computed data that should never change after construction. +//! pub computed: Frozen, +//! +//! /* some other fields */ +//! } +//! ``` +//! +//! `Frozen` impls `Deref`, so we can ergonomically call methods on `Bar`, but it doesn't `impl +//! DerefMut`. Now calling `foo.compute.mutate()` will result in a compile-time error stating that +//! `mutate` requires a mutable reference but we don't have one. + +/// An owned immutable value. +#[derive(Debug)] +pub struct Frozen(T); + +impl Frozen { + pub fn freeze(val: T) -> Self { + Frozen(val) + } +} + +impl std::ops::Deref for Frozen { + type Target = T; + + fn deref(&self) -> &T { + &self.0 + } +} diff --git a/src/librustc_data_structures/lib.rs b/src/librustc_data_structures/lib.rs index 13792a0c890..f9f8ff5303e 100644 --- a/src/librustc_data_structures/lib.rs +++ b/src/librustc_data_structures/lib.rs @@ -94,6 +94,7 @@ pub mod profiling; pub mod vec_linked_list; pub mod work_queue; pub use atomic_ref::AtomicRef; +pub mod frozen; pub struct OnDrop(pub F); diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 8dc6ee7ea3b..a61d00b0120 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -73,24 +73,6 @@ crate use place_ext::PlaceExt; crate use places_conflict::{places_conflict, PlaceConflictBias}; crate use region_infer::RegionInferenceContext; -/// An owned immutable value. -#[derive(Debug)] -struct Frozen(T); - -impl Frozen { - pub fn freeze(val: T) -> Self { - Frozen(val) - } -} - -impl std::ops::Deref for Frozen { - type Target = T; - - fn deref(&self) -> &T { - &self.0 - } -} - // FIXME(eddyb) perhaps move this somewhere more centrally. #[derive(Debug)] crate struct Upvar { @@ -1595,11 +1577,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { mpi, ); } // Only query longest prefix with a MovePath, not further - // ancestors; dataflow recurs on children when parents - // move (to support partial (re)inits). - // - // (I.e., querying parents breaks scenario 7; but may want - // to do such a query based on partial-init feature-gate.) + // ancestors; dataflow recurs on children when parents + // move (to support partial (re)inits). + // + // (I.e., querying parents breaks scenario 7; but may want + // to do such a query based on partial-init feature-gate.) } /// Subslices correspond to multiple move paths, so we iterate through the diff --git a/src/librustc_mir/borrow_check/region_infer/mod.rs b/src/librustc_mir/borrow_check/region_infer/mod.rs index 089a9b7328e..fe96b3e34a2 100644 --- a/src/librustc_mir/borrow_check/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/region_infer/mod.rs @@ -7,6 +7,7 @@ use rustc::mir::{ }; use rustc::ty::{self, subst::SubstsRef, RegionVid, Ty, TyCtxt, TypeFoldable}; use rustc_data_structures::binary_search_util; +use rustc_data_structures::frozen::Frozen; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::graph::scc::Sccs; use rustc_hir::def_id::DefId; @@ -31,7 +32,6 @@ use crate::borrow_check::{ }, type_check::{free_region_relations::UniversalRegionRelations, Locations}, universal_regions::UniversalRegions, - Frozen, }; mod dump_mir; diff --git a/src/librustc_mir/borrow_check/type_check/free_region_relations.rs b/src/librustc_mir/borrow_check/type_check/free_region_relations.rs index a18004ce191..ca61df018bc 100644 --- a/src/librustc_mir/borrow_check/type_check/free_region_relations.rs +++ b/src/librustc_mir/borrow_check/type_check/free_region_relations.rs @@ -1,6 +1,7 @@ use rustc::mir::ConstraintCategory; use rustc::ty::free_region_map::FreeRegionRelations; use rustc::ty::{self, RegionVid, Ty, TyCtxt}; +use rustc_data_structures::frozen::Frozen; use rustc_data_structures::transitive_relation::TransitiveRelation; use rustc_infer::infer::canonical::QueryRegionConstraints; use rustc_infer::infer::region_constraints::GenericKind; @@ -15,7 +16,6 @@ use crate::borrow_check::{ type_check::constraint_conversion, type_check::{Locations, MirTypeckRegionConstraints}, universal_regions::UniversalRegions, - Frozen, }; #[derive(Debug)] diff --git a/src/librustc_mir/borrow_check/type_check/mod.rs b/src/librustc_mir/borrow_check/type_check/mod.rs index f4656673fec..37d179d3d16 100644 --- a/src/librustc_mir/borrow_check/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/type_check/mod.rs @@ -18,6 +18,7 @@ use rustc::ty::{ self, CanonicalUserTypeAnnotation, CanonicalUserTypeAnnotations, RegionVid, ToPolyTraitRef, Ty, TyCtxt, UserType, UserTypeAnnotationIndex, }; +use rustc_data_structures::frozen::Frozen; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::struct_span_err; use rustc_hir as hir; @@ -55,7 +56,6 @@ use crate::borrow_check::{ renumber, type_check::free_region_relations::{CreateResult, UniversalRegionRelations}, universal_regions::{DefiningTy, UniversalRegions}, - Frozen, }; macro_rules! span_mirbug { From a58b17f2b5e57baa45ffb5b8c979faa3191bc05a Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Fri, 13 Mar 2020 13:36:16 -0500 Subject: [PATCH 4/4] update rustdocs for frozen --- src/librustc_data_structures/frozen.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/librustc_data_structures/frozen.rs b/src/librustc_data_structures/frozen.rs index 835fa7d839c..2daf5b04141 100644 --- a/src/librustc_data_structures/frozen.rs +++ b/src/librustc_data_structures/frozen.rs @@ -1,4 +1,4 @@ -//! An immutable, owned value. +//! An immutable, owned value (except for interior mutability). //! //! The purpose of `Frozen` is to make a value immutable for the sake of defensive programming. For example, //! suppose we have the following: @@ -37,6 +37,12 @@ //! `Frozen` impls `Deref`, so we can ergonomically call methods on `Bar`, but it doesn't `impl //! DerefMut`. Now calling `foo.compute.mutate()` will result in a compile-time error stating that //! `mutate` requires a mutable reference but we don't have one. +//! +//! # Caveats +//! +//! - `Frozen` doesn't try to defend against interior mutability (e.g. `Frozen>`). +//! - `Frozen` doesn't pin it's contents (e.g. one could still do `foo.computed = +//! Frozen::freeze(new_bar)`). /// An owned immutable value. #[derive(Debug)]