From 6b33f47514f47267d9e07bebbf4c97b2404fa58c Mon Sep 17 00:00:00 2001
From: Niko Matsakis <niko@alum.mit.edu>
Date: Tue, 16 Aug 2016 11:42:30 -0400
Subject: [PATCH] remove `usize: DepGraphRead` and add `Untracked`

The idea is that a `usize` is sort of ambiguous: in this case, it
represents indices that do not need tracking, but it could as easily be
some data read out from a tracked location, and hence represent tracked
data. Therefore, we add an `Untracked` type that lets user assert
that value is not tracked.

Also correct various typos.
---
 src/librustc_metadata/encoder.rs       | 25 ++++++++++++++++++-------
 src/librustc_metadata/index_builder.rs | 23 +++++++++++++++++------
 2 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs
index 7f7b87fb880b..791d090273ff 100644
--- a/src/librustc_metadata/encoder.rs
+++ b/src/librustc_metadata/encoder.rs
@@ -53,7 +53,7 @@ use rustc::hir::intravisit::Visitor;
 use rustc::hir::intravisit;
 use rustc::hir::map::DefKey;
 
-use super::index_builder::{FromId, IndexBuilder, ItemContentBuilder, XRef};
+use super::index_builder::{FromId, IndexBuilder, ItemContentBuilder, Untracked, XRef};
 
 pub struct EncodeContext<'a, 'tcx: 'a> {
     pub diag: &'a Handler,
@@ -206,15 +206,20 @@ impl<'a, 'tcx, 'encoder> IndexBuilder<'a, 'tcx, 'encoder> {
         for (i, variant) in def.variants.iter().enumerate() {
             self.record(variant.did,
                         ItemContentBuilder::encode_enum_variant_info,
-                        (enum_did, i));
+                        (enum_did, Untracked(i)));
         }
     }
 }
 
 impl<'a, 'tcx, 'encoder> ItemContentBuilder<'a, 'tcx, 'encoder> {
+    /// Encode data for the given variant of the given ADT. The
+    /// index of the variant is untracked: this is ok because we
+    /// will have to lookup the adt-def by its id, and that gives us
+    /// the right to access any information in the adt-def (including,
+    /// e.g., the length of the various vectors).
     fn encode_enum_variant_info(&mut self,
-                                (enum_did, index):
-                                (DefId, usize)) {
+                                (enum_did, Untracked(index)):
+                                (DefId, Untracked<usize>)) {
         let ecx = self.ecx;
         let def = ecx.tcx.lookup_adt_def(enum_did);
         let variant = &def.variants[index];
@@ -420,16 +425,22 @@ impl<'a, 'tcx, 'encoder> IndexBuilder<'a, 'tcx, 'encoder> {
             for (field_index, field) in variant.fields.iter().enumerate() {
                 self.record(field.did,
                             ItemContentBuilder::encode_field,
-                            (adt_def_id, variant_index, field_index));
+                            (adt_def_id, Untracked((variant_index, field_index))));
             }
         }
     }
 }
 
 impl<'a, 'tcx, 'encoder> ItemContentBuilder<'a, 'tcx, 'encoder> {
+    /// Encode data for the given field of the given variant of the
+    /// given ADT. The indices of the variant/field are untracked:
+    /// this is ok because we will have to lookup the adt-def by its
+    /// id, and that gives us the right to access any information in
+    /// the adt-def (including, e.g., the length of the various
+    /// vectors).
     fn encode_field(&mut self,
-                    (adt_def_id, variant_index, field_index):
-                    (DefId, usize, usize)) {
+                    (adt_def_id, Untracked((variant_index, field_index))):
+                    (DefId, Untracked<(usize, usize)>)) {
         let ecx = self.ecx();
         let def = ecx.tcx.lookup_adt_def(adt_def_id);
         let variant = &def.variants[variant_index];
diff --git a/src/librustc_metadata/index_builder.rs b/src/librustc_metadata/index_builder.rs
index 37f29696808e..1d3d09d6bc2d 100644
--- a/src/librustc_metadata/index_builder.rs
+++ b/src/librustc_metadata/index_builder.rs
@@ -24,10 +24,10 @@
 //!
 //! In addition to the offset, we need to track the data that was used
 //! to generate the contents of each `data_item`. This is so that we
-//! can figure out which HIR nodes contributors to that data for
+//! can figure out which HIR nodes contributed to that data for
 //! incremental compilation purposes.
 //!
-//! The `IndexBuilder` facilitates with both of these. It is created
+//! The `IndexBuilder` facilitates both of these. It is created
 //! with an RBML encoder isntance (`rbml_w`) along with an
 //! `EncodingContext` (`ecx`), which it encapsulates. It has one main
 //! method, `record()`. You invoke `record` like so to create a new
@@ -166,10 +166,6 @@ pub trait DepGraphRead {
     fn read(&self, tcx: TyCtxt);
 }
 
-impl DepGraphRead for usize {
-    fn read(&self, _tcx: TyCtxt) { }
-}
-
 impl DepGraphRead for DefId {
     fn read(&self, _tcx: TyCtxt) { }
 }
@@ -229,6 +225,21 @@ read_hir!(hir::ImplItem);
 read_hir!(hir::TraitItem);
 read_hir!(hir::ForeignItem);
 
+/// Leaks access to a value of type T without any tracking. This is
+/// suitable for ambiguous types like `usize`, which *could* represent
+/// tracked data (e.g., if you read it out of a HIR node) or might not
+/// (e.g., if it's an index). Adding in an `Untracked` is an
+/// assertion, essentially, that the data does not need to be tracked
+/// (or that read edges will be added by some other way).
+///
+/// A good idea is to add to each use of `Untracked` an explanation of
+/// why this value is ok.
+pub struct Untracked<T>(pub T);
+
+impl<T> DepGraphRead for Untracked<T> {
+    fn read(&self, _tcx: TyCtxt) { }
+}
+
 /// Newtype that can be used to package up misc data extracted from a
 /// HIR node that doesn't carry its own id. This will allow an
 /// arbitrary `T` to be passed in, but register a read on the given