Fix non-deterministic encoding of syntax contexts

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -31,7 +31,7 @@ use rustc_serialize::{Decodable, Decoder, Encodable, Encoder, opaque};
 use rustc_session::config::mitigation_coverage::DeniedPartialMitigation;
 use rustc_session::config::{OptLevel, TargetModifier};
 use rustc_span::def_id::CRATE_MOD_ID;
-use rustc_span::hygiene::{HygieneEncodeContext, raw_encode_syntax_context};
+use rustc_span::hygiene::HygieneEncodeContext;
 use rustc_span::{
     ByteSymbol, ExternalSource, FileName, SourceFile, SpanData, SpanEncoder, StableSourceFileId,
     Symbol, SyntaxContext, sym,
@@ -158,7 +158,8 @@ impl<'a, 'tcx> SpanEncoder for EncodeContext<'a, 'tcx> {
     }
 
     fn encode_syntax_context(&mut self, syntax_context: SyntaxContext) {
-        raw_encode_syntax_context(syntax_context, Rc::clone(&self.hygiene_ctxt), self)
+        let idx = self.hygiene_ctxt.borrow_mut().get_syntax_ctxt_encoding_index(syntax_context);
+        idx.encode(self);
     }
 
     fn encode_expn_id(&mut self, expn_id: ExpnId) {

compiler/rustc_middle/src/query/on_disk_cache.rs

@@ -18,7 +18,6 @@ use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
 use rustc_session::Session;
 use rustc_span::hygiene::{
     ExpnId, HygieneDecodeContext, HygieneEncodeContext, SyntaxContext, SyntaxContextKey,
-    raw_encode_syntax_context,
 };
 use rustc_span::{
     BlobDecoder, BytePos, ByteSymbol, CachingSourceMapView, ExpnData, ExpnHash, RelativeBytePos,
@@ -870,7 +869,8 @@ impl<'tcx> CacheEncoder<'tcx> {
 
 impl<'tcx> SpanEncoder for CacheEncoder<'tcx> {
     fn encode_syntax_context(&mut self, syntax_context: SyntaxContext) {
-        raw_encode_syntax_context(syntax_context, Rc::clone(&self.hygiene_context), self);
+        let idx = self.hygiene_context.borrow_mut().get_syntax_ctxt_encoding_index(syntax_context);
+        idx.encode(self);
     }
 
     fn encode_expn_id(&mut self, expn_id: ExpnId) {

compiler/rustc_span/src/hygiene.rs

@@ -26,7 +26,6 @@
 
 use std::cell::RefCell;
 use std::hash::Hash;
-use std::rc::Rc;
 use std::sync::Arc;
 use std::{fmt, iter, mem};
 
@@ -40,7 +39,7 @@ use rustc_data_structures::unhash::UnhashMap;
 use rustc_hashes::Hash64;
 use rustc_index::IndexVec;
 use rustc_macros::{Decodable, Encodable, StableHash};
-use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
+use rustc_serialize::{Decodable, Decoder, Encodable};
 use tracing::{debug, trace};
 
 use crate::def_id::{CRATE_DEF_ID, CrateNum, DefId, LOCAL_CRATE, ModId, StableCrateId};
@@ -1293,25 +1292,75 @@ impl DesugaringKind {
     }
 }
 
-#[derive(Default)]
 pub struct HygieneEncodeContext {
     /// All `SyntaxContexts` for which we have written `SyntaxContextData` into crate metadata.
     serialized_ctxts: FxHashSet<SyntaxContext>,
     /// The `SyntaxContexts` that we have serialized (e.g. as a result of encoding `Spans`)
     /// in the most recent 'round' of serializing. Serializing `SyntaxContextData`
     /// may cause us to serialize more `SyntaxContext`s, so serialize in a loop
     /// until we reach a fixed point.
-    latest_ctxts: FxHashSet<SyntaxContext>,
+    latest_ctxts: Vec<(u32 /* Encoding index */, SyntaxContext)>,
 
     serialized_expns: FxHashSet<ExpnId>,
-    latest_expns: FxHashSet<ExpnId>,
+    latest_expns: Vec<ExpnId>,
+
+    /// Maps every `SyntaxContext` into its encoding index.
+    /// Earlier the `ctxt.0` was used when writing metadata, however,
+    /// this results into non-deterministic metadata (see #129094).
+    /// The non-determinism is encountered when decoding syntax contexts
+    /// in `decode_syntax_context` function below. The syntax contexts from
+    /// other crate metadata can be decoded in different order, which results
+    /// into different ids assigned to decoded syntax contexts.
+    /// First invocation:
+    /// (ALLOC - syntax context id, ORIG - original id of decoded syntax context:
+    /// `raw_id` in `decode_syntax_context`)
+    /// ALLOC: #3, ORIG: 1
+    /// ALLOC: #9, ORIG: 18769
+    /// ALLOC: #10, ORIG: 25868
+    /// ALLOC: #11, ORIG: 18822
+    /// ALLOC: #12, ORIG: 23092
+    ///
+    /// Second invocation:
+    /// ALLOC: #3, ORIG: 1
+    /// ALLOC: #9, ORIG: 25868
+    /// ALLOC: #10, ORIG: 18769
+    /// ALLOC: #11, ORIG: 18822
+    /// ALLOC: #12, ORIG: 23092
+    ///
+    /// We see that `18769` and `25868` assigned different syntax context ids,
+    /// however, the order of encoding is deterministic, so we can remap allocated
+    /// syntax context ids into encoding indices and use them, thus outputting
+    /// same metadata.
+    encoding_indices: FxHashMap<SyntaxContext, u32>,
+}
+
+impl Default for HygieneEncodeContext {
+    fn default() -> HygieneEncodeContext {
+        HygieneEncodeContext {
+            serialized_ctxts: Default::default(),
+            latest_ctxts: Default::default(),
+            serialized_expns: Default::default(),
+            latest_expns: Default::default(),
+            // Zero is taken by root syntax context.
+            encoding_indices: FxHashMap::from_iter(iter::once((SyntaxContext::root(), 0))),
+        }
+    }
 }
 
 impl HygieneEncodeContext {
+    #[inline]
+    fn get_encoding_index(&mut self, ctxt: SyntaxContext) -> u32 {
+        let map = &mut self.encoding_indices;
+        let len = map.len();
+        *map.entry(ctxt).or_insert(len as u32)
+    }
+
     /// Record the fact that we need to serialize the corresponding `ExpnData`.
     #[inline]
     pub fn schedule_expn_data_for_encoding(&mut self, expn: ExpnId) {
-        self.latest_expns.insert(expn);
+        if self.serialized_expns.insert(expn) {
+            self.latest_expns.push(expn);
+        }
     }
 
     pub fn encode<T>(
@@ -1322,11 +1371,6 @@ impl HygieneEncodeContext {
     ) {
         // When we serialize a `SyntaxContextData`, we may end up serializing
         // a `SyntaxContext` that we haven't seen before
-
-        // Reuse the capacity between the loop iterations below.
-        let mut all_ctxt_data = vec![];
-        let mut all_expn_data = vec![];
-
         while {
             let h_ctxt = h_ctxt.borrow();
             !h_ctxt.latest_ctxts.is_empty() || !h_ctxt.latest_expns.is_empty()
@@ -1337,58 +1381,51 @@ impl HygieneEncodeContext {
                 h_ctxt.borrow().latest_ctxts
             );
 
-            let mut mut_hctxt = h_ctxt.borrow_mut();
-
             // Consume the current round of syntax contexts.
             // It's fine to iterate over a HashSet, because the serialization of the table
             // that we insert data into doesn't depend on insertion order.
             #[allow(rustc::potential_query_instability)]
-            let latest_ctxts = { mem::take(&mut mut_hctxt.latest_ctxts) }.into_iter();
+            let latest_contexts = { mem::take(&mut h_ctxt.borrow_mut().latest_ctxts) }.into_iter();
 
-            HygieneData::with(|data| {
-                for ctxt in latest_ctxts {
-                    if !mut_hctxt.serialized_ctxts.insert(ctxt) {
-                        continue;
-                    }
-
-                    all_ctxt_data.push((ctxt.0, data.syntax_context_data[ctxt.0 as usize].key()));
-                }
-            });
-
-            drop(mut_hctxt);
-
-            for (idx, ctxt_key) in all_ctxt_data.drain(..) {
-                encode_ctxt(encoder, idx, &ctxt_key);
+            for (idx, ctxt) in latest_contexts {
+                let key = HygieneData::with(|data| data.syntax_context_data[ctxt.0 as usize].key());
+                encode_ctxt(encoder, idx, &key);
             }
 
-            let mut mut_hctxt = h_ctxt.borrow_mut();
-
             // Same as above, but for expansions instead of syntax contexts.
             #[allow(rustc::potential_query_instability)]
-            let latest_expns = { mem::take(&mut mut_hctxt.latest_expns) }.into_iter();
-            HygieneData::with(|data| {
-                for expn in latest_expns {
-                    if !mut_hctxt.serialized_expns.insert(expn) {
-                        continue;
-                    }
-
-                    // We need `data` only for local expansions, so don't `data` for non-local
+            let latest_expns = { mem::take(&mut h_ctxt.borrow_mut().latest_expns) }.into_iter();
+
+            for expn in latest_expns {
+                let (data, hash) = HygieneData::with(|data| {
+                    // We need `data` only for local expansions, so don't clone `data` for non-local
                     // expansions.
                     // FIXME: completely remove this clone
                     let expn_data = expn.as_local().map(|id| data.local_expn_data(id).clone());
-                    all_expn_data.push((expn, expn_data, data.expn_hash(expn)));
-                }
-            });
-
-            drop(mut_hctxt);
+                    (expn_data, data.expn_hash(expn))
+                });
 
-            for (expn, expn_data, expn_hash) in all_expn_data.drain(..) {
-                encode_expn(encoder, expn, expn_data.as_ref(), expn_hash);
+                encode_expn(encoder, expn, data.as_ref(), hash);
             }
         }
 
         debug!("encode_hygiene: Done serializing SyntaxContextData");
     }
+
+    #[inline]
+    pub fn get_syntax_ctxt_encoding_index(&mut self, ctxt: SyntaxContext) -> u32 {
+        let index = self.get_encoding_index(ctxt);
+        if self.serialized_ctxts.insert(ctxt) {
+            // If we created new encoding index then it is greater
+            // than any previous index, so this vector is in ascending order.
+            // We can't push existing, possibly out-of-order, index
+            // as we check if we already saw this syntax context above.
+            // This property is important for deterministic output (see #129094).
+            self.latest_ctxts.push((index, ctxt));
+        }
+
+        index
+    }
 }
 
 /// Additional information used to assist in decoding hygiene data
@@ -1509,16 +1546,6 @@ impl<D: SpanDecoder> Decodable<D> for LocalExpnId {
     }
 }
 
-#[inline]
-pub fn raw_encode_syntax_context(
-    ctxt: SyntaxContext,
-    context: Rc<RefCell<HygieneEncodeContext>>,
-    e: &mut impl Encoder,
-) {
-    context.borrow_mut().latest_ctxts.insert(ctxt);
-    ctxt.0.encode(e);
-}
-
 /// Updates the `disambiguator` field of the corresponding `ExpnData`
 /// such that the `Fingerprint` of the `ExpnData` does not collide with
 /// any other `ExpnIds`.

tests/run-make/parallel-reproducible-build/derives-issue-129094.rs

@@ -0,0 +1,5 @@
+#![crate_type = "lib"]
+#[derive(Clone, Copy, Hash, PartialEq, PartialOrd)]
+struct PackedPoint {
+    x: u32,
+}

tests/run-make/parallel-reproducible-build/rmake.rs

@@ -7,29 +7,38 @@ use std::rc::Rc;
 
 use run_make_support::{bin_name, is_windows_msvc, rfs, run_in_tmpdir, rustc};
 
-/// Test that parallel compiler produces identical binaries.
+/// Test that parallel compiler produces identical artifacts (binaries, metadata).
 fn main() {
-    const FILE_NAME: &str = "static-muts-issue-140413";
-    let bin_name = bin_name(FILE_NAME);
+    const TESTS: &[(&str, &[&str])] = &[
+        ("static-muts-issue-140413", &["-Zthreads=50"]),
+        ("derives-issue-129094", &["-Zthreads=16", "-Copt-level=3"]),
+    ];
 
-    let mut reference = None;
+    for (file, args) in TESTS {
+        let mut reference = None;
+        let bin_name = bin_name(file);
 
-    for _ in 0..10 {
-        // Tmp dir as previous runs affect output binary on windows.
-        run_in_tmpdir(|| {
-            let mut rustc = rustc();
-            rustc.input(format!("{FILE_NAME}.rs")).arg("-Zthreads=50").output(&bin_name);
+        for _ in 0..10 {
+            // Tmp dir as previous runs affect output binary on windows.
+            run_in_tmpdir(|| {
+                let mut rustc = rustc();
+                rustc.input(format!("{file}.rs")).output(&bin_name);
 
-            if is_windows_msvc() {
-                rustc.arg("-Clink-arg=/Brepro");
-            }
+                for arg in *args {
+                    rustc.arg(arg);
+                }
 
-            rustc.run();
+                if is_windows_msvc() {
+                    rustc.arg("-Clink-arg=/Brepro");
+                }
 
-            let current = Rc::new(rfs::read(&bin_name));
-            reference.get_or_insert(Rc::clone(&current));
+                rustc.run();
 
-            assert_eq!(Some(current), reference);
-        });
+                let current = Rc::new(rfs::read(&bin_name));
+                reference.get_or_insert(Rc::clone(&current));
+
+                assert_eq!(Some(current), reference);
+            });
+        }
     }
 }