Auto merge of #47177 - michaelwoerister:erase-invalid-spans-during-metadata-encoding, r=alexcrichton

Map invalid Spans to DUMMY_SP during crate metadata encoding.

This mirrors what we do for stabilizing the incr. comp. cache and is necessary for reproducible builds. For the incr. comp. cache, we *have* to do this because the encoding there cannot represent broken Spans. Metadata encoding has to be in sync with that as not to get unexpected interactions when compiling incrementally.

This should help with fixing issue https://github.com/rust-lang/rust/issues/47066.

r? @alexcrichton

src/librustc_metadata/decoder.rs

@@ -270,19 +270,17 @@ impl<'a, 'tcx> SpecializedDecoder<DefIndex> for DecodeContext<'a, 'tcx> {
 
 impl<'a, 'tcx> SpecializedDecoder<Span> for DecodeContext<'a, 'tcx> {
     fn specialized_decode(&mut self) -> Result<Span, Self::Error> {
+        let tag = u8::decode(self)?;
+
+        if tag == TAG_INVALID_SPAN {
+            return Ok(DUMMY_SP)
+        }
+
+        debug_assert_eq!(tag, TAG_VALID_SPAN);
+
         let lo = BytePos::decode(self)?;
-        let hi = BytePos::decode(self)?;
+        let len = BytePos::decode(self)?;
-
+        let hi = lo + len;
-        if lo == BytePos(0) && hi == BytePos(0) {
-            // Don't try to rebase DUMMY_SP. Otherwise it will look like a valid
-            // Span again.
-            return Ok(DUMMY_SP)
-        }
-
-        if hi < lo {
-            // Consistently map invalid spans to DUMMY_SP.
-            return Ok(DUMMY_SP)
-        }
 
         let sess = if let Some(sess) = self.sess {
             sess
@@ -297,9 +295,7 @@ impl<'a, 'tcx> SpecializedDecoder<Span> for DecodeContext<'a, 'tcx> {
             let last_filemap = &imported_filemaps[self.last_filemap_index];
 
             if lo >= last_filemap.original_start_pos &&
-               lo <= last_filemap.original_end_pos &&
+               lo <= last_filemap.original_end_pos {
-               hi >= last_filemap.original_start_pos &&
-               hi <= last_filemap.original_end_pos {
                 last_filemap
             } else {
                 let mut a = 0;
@@ -323,11 +319,9 @@ impl<'a, 'tcx> SpecializedDecoder<Span> for DecodeContext<'a, 'tcx> {
         debug_assert!(lo >= filemap.original_start_pos &&
                       lo <= filemap.original_end_pos);
 
-        if hi < filemap.original_start_pos || hi > filemap.original_end_pos {
+        // Make sure we correctly filtered out invalid spans during encoding
-            // `hi` points to a different FileMap than `lo` which is invalid.
+        debug_assert!(hi >= filemap.original_start_pos &&
-            // Again, map invalid Spans to DUMMY_SP.
+                      hi <= filemap.original_end_pos);
-            return Ok(DUMMY_SP)
-        }
 
         let lo = (lo + filemap.translated_filemap.start_pos) - filemap.original_start_pos;
         let hi = (hi + filemap.translated_filemap.start_pos) - filemap.original_start_pos;

src/librustc_metadata/encoder.rs

@@ -41,7 +41,7 @@ use syntax::ast::{self, CRATE_NODE_ID};
 use syntax::codemap::Spanned;
 use syntax::attr;
 use syntax::symbol::Symbol;
-use syntax_pos::{self, FileName};
+use syntax_pos::{self, FileName, FileMap, Span, DUMMY_SP};
 
 use rustc::hir::{self, PatKind};
 use rustc::hir::itemlikevisit::ItemLikeVisitor;
@@ -57,6 +57,9 @@ pub struct EncodeContext<'a, 'tcx: 'a> {
     lazy_state: LazyState,
     type_shorthands: FxHashMap<Ty<'tcx>, usize>,
     predicate_shorthands: FxHashMap<ty::Predicate<'tcx>, usize>,
+
+    // This is used to speed up Span encoding.
+    filemap_cache: Rc<FileMap>,
 }
 
 macro_rules! encoder_methods {
@@ -140,6 +143,41 @@ impl<'a, 'tcx> SpecializedEncoder<DefIndex> for EncodeContext<'a, 'tcx> {
     }
 }
 
+impl<'a, 'tcx> SpecializedEncoder<Span> for EncodeContext<'a, 'tcx> {
+    fn specialized_encode(&mut self, span: &Span) -> Result<(), Self::Error> {
+        if *span == DUMMY_SP {
+            return TAG_INVALID_SPAN.encode(self)
+        }
+
+        let span = span.data();
+
+        // The Span infrastructure should make sure that this invariant holds:
+        debug_assert!(span.lo <= span.hi);
+
+        if !self.filemap_cache.contains(span.lo) {
+            let codemap = self.tcx.sess.codemap();
+            let filemap_index = codemap.lookup_filemap_idx(span.lo);
+            self.filemap_cache = codemap.files()[filemap_index].clone();
+        }
+
+        if !self.filemap_cache.contains(span.hi) {
+            // Unfortunately, macro expansion still sometimes generates Spans
+            // that malformed in this way.
+            return TAG_INVALID_SPAN.encode(self)
+        }
+
+        TAG_VALID_SPAN.encode(self)?;
+        span.lo.encode(self)?;
+
+        // Encode length which is usually less than span.hi and profits more
+        // from the variable-length integer encoding that we use.
+        let len = span.hi - span.lo;
+        len.encode(self)
+
+        // Don't encode the expansion context.
+    }
+}
+
 impl<'a, 'tcx> SpecializedEncoder<Ty<'tcx>> for EncodeContext<'a, 'tcx> {
     fn specialized_encode(&mut self, ty: &Ty<'tcx>) -> Result<(), Self::Error> {
         ty_codec::encode_with_shorthand(self, ty, |ecx| &mut ecx.type_shorthands)
@@ -1648,6 +1686,7 @@ pub fn encode_metadata<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
             lazy_state: LazyState::NoNode,
             type_shorthands: Default::default(),
             predicate_shorthands: Default::default(),
+            filemap_cache: tcx.sess.codemap().files()[0].clone(),
         };
 
         // Encode the rustc version string in a predictable location.

src/librustc_metadata/schema.rs

@@ -521,3 +521,7 @@ pub struct GeneratorData<'tcx> {
     pub layout: mir::GeneratorLayout<'tcx>,
 }
 impl_stable_hash_for!(struct GeneratorData<'tcx> { layout });
+
+// Tags used for encoding Spans:
+pub const TAG_VALID_SPAN: u8 = 0;
+pub const TAG_INVALID_SPAN: u8 = 1;