]> git.lizzy.rs Git - rust.git/commitdiff
Simplify Scope/ScopeData to have less chance of introducing UB or size increases
authorOliver Schneider <github35764891676564198441@oli-obk.de>
Mon, 10 Sep 2018 12:40:12 +0000 (14:40 +0200)
committerOliver Schneider <github35764891676564198441@oli-obk.de>
Tue, 11 Sep 2018 09:27:12 +0000 (11:27 +0200)
src/librustc/ich/impls_ty.rs
src/librustc/infer/error_reporting/mod.rs
src/librustc/middle/region.rs
src/librustc/util/ppaux.rs
src/librustc_data_structures/indexed_vec.rs
src/librustc_mir/build/cfg.rs
src/librustc_mir/build/scope.rs
src/librustc_mir/hair/cx/block.rs

index 16175e159dd23e81baa6ca51e5eea10f8ea6065f..2bf1c79c8a43606dc5e0c120d359da8252e8f13c 100644 (file)
@@ -772,7 +772,15 @@ fn hash_stable<W: StableHasherResult>(&self,
     FnPtrAddrCast
 });
 
-impl_stable_hash_for!(struct ::middle::region::Scope { id, code });
+impl_stable_hash_for!(struct ::middle::region::Scope { id, data });
+
+impl_stable_hash_for!(enum ::middle::region::ScopeData {
+    Node,
+    CallSite,
+    Arguments,
+    Destruction,
+    Remainder(first_statement_index)
+});
 
 impl<'a> ToStableHashKey<StableHashingContext<'a>> for region::Scope {
     type KeyType = region::Scope;
@@ -783,11 +791,6 @@ fn to_stable_hash_key(&self, _: &StableHashingContext<'a>) -> region::Scope {
     }
 }
 
-impl_stable_hash_for!(struct ::middle::region::BlockRemainder {
-    block,
-    first_statement_index
-});
-
 impl_stable_hash_for!(struct ty::adjustment::CoerceUnsizedInfo {
     custom_kind
 });
index eabcf1ce4136361878b3879dc62df4271eee517d..a0c96554c91f1d4046d0ac1206eaaca6f6852ad8 100644 (file)
@@ -119,17 +119,17 @@ pub fn note_and_explain_region(
                     }
                 };
                 let scope_decorated_tag = match scope.data() {
-                    region::ScopeData::Node(_) => tag,
-                    region::ScopeData::CallSite(_) => "scope of call-site for function",
-                    region::ScopeData::Arguments(_) => "scope of function body",
-                    region::ScopeData::Destruction(_) => {
+                    region::ScopeData::Node => tag,
+                    region::ScopeData::CallSite => "scope of call-site for function",
+                    region::ScopeData::Arguments => "scope of function body",
+                    region::ScopeData::Destruction => {
                         new_string = format!("destruction scope surrounding {}", tag);
                         &new_string[..]
                     }
-                    region::ScopeData::Remainder(r) => {
+                    region::ScopeData::Remainder(first_statement_index) => {
                         new_string = format!(
                             "block suffix following statement {}",
-                            r.first_statement_index.index()
+                            first_statement_index.index()
                         );
                         &new_string[..]
                     }
index 0fbd74e250e714b7a72d5974975dad9267e30b87..6fa5d363ffa7a6426c3ed28a59ae6943446a9583 100644 (file)
@@ -20,7 +20,6 @@
 use util::nodemap::{FxHashMap, FxHashSet};
 use ty;
 
-use std::fmt;
 use std::mem;
 use rustc_data_structures::sync::Lrc;
 use syntax::source_map;
 /// placate the same deriving in `ty::FreeRegion`, but we may want to
 /// actually attach a more meaningful ordering to scopes than the one
 /// generated via deriving here.
-///
-/// Scope is a bit-packed to save space - if `code` is SCOPE_DATA_REMAINDER_MAX
-/// or less, it is a `ScopeData::Remainder`, otherwise it is a type specified
-/// by the bitpacking.
-#[derive(Clone, PartialEq, PartialOrd, Eq, Ord, Hash, Copy, RustcEncodable, RustcDecodable)]
+#[derive(Clone, PartialEq, PartialOrd, Eq, Ord, Hash, Debug, Copy, RustcEncodable, RustcDecodable)]
 pub struct Scope {
     pub(crate) id: hir::ItemLocalId,
-    pub(crate) code: u32
+    pub(crate) data: ScopeData,
 }
 
-const SCOPE_DATA_NODE: u32 = 0xFFFF_FFFF;
-const SCOPE_DATA_CALLSITE: u32 = 0xFFFF_FFFE;
-const SCOPE_DATA_ARGUMENTS: u32 = 0xFFFF_FFFD;
-const SCOPE_DATA_DESTRUCTION: u32 = 0xFFFF_FFFC;
-// be sure to add the MAX of FirstStatementIndex if you add more constants here
-
 #[derive(Clone, PartialEq, PartialOrd, Eq, Ord, Hash, Debug, Copy, RustcEncodable, RustcDecodable)]
 pub enum ScopeData {
-    Node(hir::ItemLocalId),
+    Node,
 
     // Scope of the call-site for a function or closure
     // (outlives the arguments as well as the body).
-    CallSite(hir::ItemLocalId),
+    CallSite,
 
     // Scope of arguments passed to a function or closure
     // (they outlive its body).
-    Arguments(hir::ItemLocalId),
+    Arguments,
 
     // Scope of destructors for temporaries of node-id.
-    Destruction(hir::ItemLocalId),
+    Destruction,
 
     // Scope following a `let id = expr;` binding in a block.
-    Remainder(BlockRemainder)
+    Remainder(FirstStatementIndex)
 }
 
 /// Represents a subscope of `block` for a binding that is introduced
@@ -152,12 +141,6 @@ pub enum ScopeData {
 ///
 /// * the subscope with `first_statement_index == 1` is scope of `c`,
 ///   and thus does not include EXPR_2, but covers the `...`.
-#[derive(Clone, PartialEq, PartialOrd, Eq, Ord, Hash, RustcEncodable,
-         RustcDecodable, Debug, Copy)]
-pub struct BlockRemainder {
-    pub block: hir::ItemLocalId,
-    pub first_statement_index: FirstStatementIndex,
-}
 
 newtype_index! {
     pub struct FirstStatementIndex;
@@ -165,68 +148,54 @@ pub struct BlockRemainder {
 
 impl_stable_hash_for!(struct ::middle::region::FirstStatementIndex { private });
 
-impl From<ScopeData> for Scope {
-    #[inline]
-    fn from(scope_data: ScopeData) -> Self {
-        let (id, code) = match scope_data {
-            ScopeData::Node(id) => (id, SCOPE_DATA_NODE),
-            ScopeData::CallSite(id) => (id, SCOPE_DATA_CALLSITE),
-            ScopeData::Arguments(id) => (id, SCOPE_DATA_ARGUMENTS),
-            ScopeData::Destruction(id) => (id, SCOPE_DATA_DESTRUCTION),
-            ScopeData::Remainder(r) => (r.block, r.first_statement_index.index() as u32)
-        };
-        Self { id, code }
-    }
-}
-
-impl fmt::Debug for Scope {
-    fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
-        fmt::Debug::fmt(&self.data(), formatter)
-    }
-}
+// compilation error if size of `ScopeData` is not the same as a `u32`
+#[allow(dead_code)]
+// only works on stage 1 when the rustc_layout_scalar_valid_range attribute actually exists
+#[cfg(not(stage0))]
+static ASSERT: () = [()][(mem::size_of::<ScopeData>() != 4) as usize];
 
 #[allow(non_snake_case)]
 impl Scope {
     #[inline]
     pub fn data(self) -> ScopeData {
-        match self.code {
-            SCOPE_DATA_NODE => ScopeData::Node(self.id),
-            SCOPE_DATA_CALLSITE => ScopeData::CallSite(self.id),
-            SCOPE_DATA_ARGUMENTS => ScopeData::Arguments(self.id),
-            SCOPE_DATA_DESTRUCTION => ScopeData::Destruction(self.id),
-            idx => ScopeData::Remainder(BlockRemainder {
-                block: self.id,
-                first_statement_index: FirstStatementIndex::new(idx as usize)
-            })
+        self.data
         }
+
+    #[inline]
+    pub fn new(id: hir::ItemLocalId, data: ScopeData) -> Self {
+        Scope { id, data }
     }
 
     #[inline]
     pub fn Node(id: hir::ItemLocalId) -> Self {
-        Self::from(ScopeData::Node(id))
+        Self::new(id, ScopeData::Node)
     }
 
     #[inline]
     pub fn CallSite(id: hir::ItemLocalId) -> Self {
-        Self::from(ScopeData::CallSite(id))
+        Self::new(id, ScopeData::CallSite)
     }
 
     #[inline]
     pub fn Arguments(id: hir::ItemLocalId) -> Self {
-        Self::from(ScopeData::Arguments(id))
+        Self::new(id, ScopeData::Arguments)
     }
 
     #[inline]
     pub fn Destruction(id: hir::ItemLocalId) -> Self {
-        Self::from(ScopeData::Destruction(id))
+        Self::new(id, ScopeData::Destruction)
     }
 
     #[inline]
-    pub fn Remainder(r: BlockRemainder) -> Self {
-        Self::from(ScopeData::Remainder(r))
+    pub fn Remainder(
+        id: hir::ItemLocalId,
+        first: FirstStatementIndex,
+    ) -> Self {
+        Self::new(id, ScopeData::Remainder(first))
     }
 }
 
+
 impl Scope {
     /// Returns a item-local id associated with this scope.
     ///
@@ -257,7 +226,7 @@ pub fn span(&self, tcx: TyCtxt, scope_tree: &ScopeTree) -> Span {
             return DUMMY_SP;
         }
         let span = tcx.hir.span(node_id);
-        if let ScopeData::Remainder(r) = self.data() {
+        if let ScopeData::Remainder(first_statement_index) = self.data() {
             if let Node::Block(ref blk) = tcx.hir.get(node_id) {
                 // Want span for scope starting after the
                 // indexed statement and ending at end of
@@ -267,7 +236,7 @@ pub fn span(&self, tcx: TyCtxt, scope_tree: &ScopeTree) -> Span {
                 // (This is the special case aluded to in the
                 // doc-comment for this method)
 
-                let stmt_span = blk.stmts[r.first_statement_index.index()].span;
+                let stmt_span = blk.stmts[first_statement_index.index()].span;
 
                 // To avoid issues with macro-generated spans, the span
                 // of the statement must be nested in that of the block.
@@ -511,8 +480,8 @@ pub fn record_scope_parent(&mut self, child: Scope, parent: Option<(Scope, Scope
         }
 
         // record the destruction scopes for later so we can query them
-        if let ScopeData::Destruction(n) = child.data() {
-            self.destruction_scopes.insert(n, child);
+        if let ScopeData::Destruction = child.data() {
+            self.destruction_scopes.insert(child.item_local_id(), child);
         }
     }
 
@@ -595,7 +564,7 @@ pub fn temporary_scope(&self, expr_id: hir::ItemLocalId) -> Option<Scope> {
 
         while let Some(&(p, _)) = self.parent_map.get(&id) {
             match p.data() {
-                ScopeData::Destruction(..) => {
+                ScopeData::Destruction => {
                     debug!("temporary_scope({:?}) = {:?} [enclosing]",
                            expr_id, id);
                     return Some(id);
@@ -650,8 +619,8 @@ pub fn is_subscope_of(&self,
     /// Returns the id of the innermost containing body
     pub fn containing_body(&self, mut scope: Scope)-> Option<hir::ItemLocalId> {
         loop {
-            if let ScopeData::CallSite(id) = scope.data() {
-                return Some(id);
+            if let ScopeData::CallSite = scope.data() {
+                return Some(scope.item_local_id());
             }
 
             match self.opt_encl_scope(scope) {
@@ -867,10 +836,7 @@ fn resolve_block<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, blk:
                 // except for the first such subscope, which has the
                 // block itself as a parent.
                 visitor.enter_scope(
-                    Scope::Remainder(BlockRemainder {
-                        block: blk.hir_id.local_id,
-                        first_statement_index: FirstStatementIndex::new(i)
-                    })
+                    Scope::Remainder(blk.hir_id.local_id, FirstStatementIndex::new(i))
                 );
                 visitor.cx.var_parent = visitor.cx.parent;
             }
@@ -1033,7 +999,7 @@ fn resolve_expr<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, expr:
             match visitor.scope_tree.parent_map.get(&scope) {
                 // Don't cross from closure bodies to their parent.
                 Some(&(superscope, _)) => match superscope.data() {
-                    ScopeData::CallSite(_) => break,
+                    ScopeData::CallSite => break,
                     _ => scope = superscope
                 },
                 None => break
index ddcc0fa9c9280d0ed609e2e93fe8eb05ae9cb822..1206690c86e24dd6bf2044ff61521bd41711d166 100644 (file)
@@ -11,7 +11,7 @@
 use hir::def_id::DefId;
 use hir::map::definitions::DefPathData;
 use mir::interpret::ConstValue;
-use middle::region::{self, BlockRemainder};
+use middle::region;
 use ty::subst::{self, Subst};
 use ty::{BrAnon, BrEnv, BrFresh, BrNamed};
 use ty::{Bool, Char, Adt};
@@ -770,17 +770,20 @@ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
                 }
                 ty::ReScope(scope) if cx.identify_regions => {
                     match scope.data() {
-                        region::ScopeData::Node(id) =>
-                            write!(f, "'{}s", id.as_usize()),
-                        region::ScopeData::CallSite(id) =>
-                            write!(f, "'{}cs", id.as_usize()),
-                        region::ScopeData::Arguments(id) =>
-                            write!(f, "'{}as", id.as_usize()),
-                        region::ScopeData::Destruction(id) =>
-                            write!(f, "'{}ds", id.as_usize()),
-                        region::ScopeData::Remainder(BlockRemainder
-                                                     { block, first_statement_index }) =>
-                            write!(f, "'{}_{}rs", block.as_usize(), first_statement_index.index()),
+                        region::ScopeData::Node =>
+                            write!(f, "'{}s", scope.item_local_id().as_usize()),
+                        region::ScopeData::CallSite =>
+                            write!(f, "'{}cs", scope.item_local_id().as_usize()),
+                        region::ScopeData::Arguments =>
+                            write!(f, "'{}as", scope.item_local_id().as_usize()),
+                        region::ScopeData::Destruction =>
+                            write!(f, "'{}ds", scope.item_local_id().as_usize()),
+                        region::ScopeData::Remainder(first_statement_index) => write!(
+                            f,
+                            "'{}_{}rs",
+                            scope.item_local_id().as_usize(),
+                            first_statement_index.index()
+                        ),
                     }
                 }
                 ty::ReVar(region_vid) if cx.identify_regions => {
index 0b5fb97f0e1fc7a2d6fa7532f7613124318b99b8..2f11fea46d69a6c6db09c10ea0a5c0d32e4a6546 100644 (file)
@@ -72,7 +72,8 @@ macro_rules! newtype_index {
         newtype_index!(
             // Leave out derives marker so we can use its absence to ensure it comes first
             @type         [$name]
-            @max          [0xFFFF_FFFE]
+            // shave off 256 indices at the end to allow space for packing these indices into enums
+            @max          [0xFFFF_FF00]
             @vis          [$v]
             @debug_format ["{}"]);
     );
@@ -82,7 +83,8 @@ macro_rules! newtype_index {
         newtype_index!(
             // Leave out derives marker so we can use its absence to ensure it comes first
             @type         [$name]
-            @max          [0xFFFF_FFFE]
+            // shave off 256 indices at the end to allow space for packing these indices into enums
+            @max          [0xFFFF_FF00]
             @vis          [$v]
             @debug_format ["{}"]
                           $($tokens)+);
index 1ed8289d4418469544d5cd7400a6b718584516c6..d46b0813ca703e840c6fb4331fbd470a78e5bc79 100644 (file)
@@ -51,7 +51,7 @@ pub fn push_end_region<'a, 'gcx:'a+'tcx>(&mut self,
                                              source_info: SourceInfo,
                                              region_scope: region::Scope) {
         if tcx.emit_end_regions() {
-            if let region::ScopeData::CallSite(_) = region_scope.data() {
+            if let region::ScopeData::CallSite = region_scope.data() {
                 // The CallSite scope (aka the root scope) is sort of weird, in that it is
                 // supposed to "separate" the "interior" and "exterior" of a closure. Being
                 // that, it is not really a part of the region hierarchy, but for some
index 38e0854bcd61eddabdd00c694f9efc45018d9a43..1406183955bd430ce962fc556893714165d9d557 100644 (file)
@@ -566,7 +566,7 @@ pub fn region_scope_of_return_scope(&self) -> region::Scope {
         // We want `scopes[1]`, which is the `ParameterScope`.
         assert!(self.scopes.len() >= 2);
         assert!(match self.scopes[1].region_scope.data() {
-            region::ScopeData::Arguments(_) => true,
+            region::ScopeData::Arguments => true,
             _ => false,
         });
         self.scopes[1].region_scope
index 6c8b5d97b6ff314abb58ec95be28d7ed02c9998e..730603dff56ff7d1f19f77974c5b28b7b8623c69 100644 (file)
@@ -11,7 +11,7 @@
 use hair::*;
 use hair::cx::Cx;
 use hair::cx::to_ref::ToRef;
-use rustc::middle::region::{self, BlockRemainder};
+use rustc::middle::region;
 use rustc::hir;
 
 use rustc_data_structures::indexed_vec::Idx;
@@ -71,10 +71,10 @@ fn mirror_stmts<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
                         // ignore for purposes of the MIR
                     }
                     hir::DeclKind::Local(ref local) => {
-                        let remainder_scope = region::Scope::Remainder(BlockRemainder {
-                            block: block_id,
-                            first_statement_index: region::FirstStatementIndex::new(index),
-                        });
+                        let remainder_scope = region::Scope::Remainder(
+                            block_id,
+                            region::FirstStatementIndex::new(index),
+                        );
 
                         let ty = local.ty.clone().map(|ty| ty.hir_id);
                         let pattern = cx.pattern_from_hir(&local.pat);