Address comments

src/concurrency/data_race.rs

@@ -200,15 +200,13 @@ enum AtomicAccessType {
     Rmw,
 }
 
-/// Type of write operation: allocating memory
-/// non-atomic writes and deallocating memory
-/// are all treated as writes for the purpose
-/// of the data-race detector.
+/// Type of read operation
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
 pub enum NaReadType {
     /// Standard unsynchronized write.
     Read,
 
+    // An implicit read generated by a retag.
     Retag,
 }
 
@@ -233,6 +231,7 @@ pub enum NaWriteType {
     /// Standard unsynchronized write.
     Write,
 
+    // An implicit write generated by a retag.
     Retag,
 
     /// Deallocate memory.
@@ -279,7 +278,7 @@ impl AccessType {
         });
 
         if let Some(ty) = ty {
-            msg.push_str(&format!(" with type `{}`", ty));
+            msg.push_str(&format!(" of type `{}`", ty));
         }
 
         msg
@@ -920,7 +919,8 @@ impl VClockAlloc {
     /// This finds the two racing threads and the type
     /// of data-race that occurred. This will also
     /// return info about the memory location the data-race
-    /// occurred in.
+    /// occurred in. The `ty` parameter is used for diagnostics, letting
+    /// the user know which type was involved in the access.
     #[cold]
     #[inline(never)]
     fn report_data_race<'tcx>(
@@ -1011,11 +1011,12 @@ impl VClockAlloc {
         }))?
     }
 
-    /// Detect data-races for an unsynchronized read operation, will not perform
+    /// Detect data-races for an unsynchronized read operation. It will not perform
     /// data-race detection if `race_detecting()` is false, either due to no threads
     /// being created or if it is temporarily disabled during a racy read or write
     /// operation for which data-race detection is handled separately, for example
-    /// atomic read operations.
+    /// atomic read operations. The `ty` parameter is used for diagnostics, letting
+    /// the user know which type was read.
     pub fn read<'tcx>(
         &self,
         alloc_id: AllocId,
@@ -1054,10 +1055,11 @@ impl VClockAlloc {
         }
     }
 
-    /// Detect data-races for an unsynchronized write operation, will not perform
-    /// data-race threads if `race_detecting()` is false, either due to no threads
+    /// Detect data-races for an unsynchronized write operation. It will not perform
+    /// data-race detection if `race_detecting()` is false, either due to no threads
     /// being created or if it is temporarily disabled during a racy read or write
-    /// operation
+    /// operation. The `ty` parameter is used for diagnostics, letting
+    /// the user know which type was written.
     pub fn write<'tcx>(
         &mut self,
         alloc_id: AllocId,

src/concurrency/vector_clock.rs

@@ -52,25 +52,33 @@ const SMALL_VECTOR: usize = 4;
 /// so that diagnostics can report what code was responsible for an operation.
 #[derive(Clone, Copy, Debug)]
 pub struct VTimestamp {
-    // The lowest bit indicates read type, the rest is the time.
+    /// The lowest bit indicates read type, the rest is the time.
+    /// `1` indicates a retag read, `0` a regular read.
     time_and_read_type: u32,
     pub span: Span,
 }
 
 impl VTimestamp {
-    pub const ZERO: VTimestamp = VTimestamp { time_and_read_type: 0, span: DUMMY_SP };
+    pub const ZERO: VTimestamp = VTimestamp::new(0, NaReadType::Read, DUMMY_SP);
+
+    const fn new(time: u32, read_type: NaReadType, span: Span) -> Self {
+        let read_type = match read_type {
+            NaReadType::Read => 0,
+            NaReadType::Retag => 1,
+        };
+        Self {
+            time_and_read_type: read_type | time.checked_mul(2).expect("Vector clock overflow"),
+            span,
+        }
+    }
 
     fn time(&self) -> u32 {
         self.time_and_read_type.shr(1)
     }
 
     fn set_time(&mut self, time: u32) {
         self.time_and_read_type =
-            (self.time_and_read_type & 1) | time.checked_shl(1).expect("Vector clock overflow")
-    }
-
-    pub fn span_data(&self) -> SpanData {
-        self.span.data()
+            (self.time_and_read_type & 1) | time.checked_mul(2).expect("Vector clock overflow")
     }
 
     pub fn read_type(&self) -> NaReadType {
@@ -80,6 +88,10 @@ impl VTimestamp {
     pub fn set_read_type(&mut self, read_type: NaReadType) {
         self.time_and_read_type = (self.time_and_read_type & !1) | read_type as u32
     }
+
+    pub fn span_data(&self) -> SpanData {
+        self.span.data()
+    }
 }
 
 impl PartialEq for VTimestamp {
@@ -400,8 +412,9 @@ impl IndexMut<VectorIdx> for VClock {
 #[cfg(test)]
 mod tests {
     use super::{VClock, VTimestamp, VectorIdx};
+    use crate::concurrency::data_race::NaReadType;
     use rustc_span::DUMMY_SP;
-    use std::{cmp::Ordering, ops::Shl};
+    use std::cmp::Ordering;
 
     #[test]
     fn test_equal() {
@@ -471,7 +484,7 @@ mod tests {
             slice
                 .iter()
                 .copied()
-                .map(|time| VTimestamp { time_and_read_type: time.shl(1), span: DUMMY_SP })
+                .map(|time| VTimestamp::new(time, NaReadType::Read, DUMMY_SP))
                 .collect(),
         )
     }

src/lib.rs

@@ -1,5 +1,6 @@
 #![feature(rustc_private)]
 #![feature(cell_update)]
+#![feature(const_option)]
 #![feature(float_gamma)]
 #![feature(generic_nonzero)]
 #![feature(map_try_insert)]

tests/fail/stacked_borrows/retag_data_race_protected_read.rs

@@ -13,7 +13,7 @@ fn main() {
         let ptr = ptr;
         // We do a protected mutable retag (but no write!) in this thread.
         fn retag(_x: &mut i32) {}
-        retag(unsafe { &mut *ptr.0 }); //~ERROR: Data race detected between (1) non-atomic read on thread `main` and (2) retag write with type `i32` on thread `unnamed-1`
+        retag(unsafe { &mut *ptr.0 }); //~ERROR: Data race detected between (1) non-atomic read on thread `main` and (2) retag write of type `i32` on thread `unnamed-1`
     });
 
     // We do a read in the main thread.

tests/fail/stacked_borrows/retag_data_race_protected_read.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: Data race detected between (1) non-atomic read on thread `main` and (2) retag write with type `i32` on thread `unnamed-ID` at ALLOC. (2) just happened here
+error: Undefined Behavior: Data race detected between (1) non-atomic read on thread `main` and (2) retag write of type `i32` on thread `unnamed-ID` at ALLOC. (2) just happened here
   --> $DIR/retag_data_race_protected_read.rs:LL:CC
    |
 LL |         retag(unsafe { &mut *ptr.0 });
-   |                        ^^^^^^^^^^^ Data race detected between (1) non-atomic read on thread `main` and (2) retag write with type `i32` on thread `unnamed-ID` at ALLOC. (2) just happened here
+   |                        ^^^^^^^^^^^ Data race detected between (1) non-atomic read on thread `main` and (2) retag write of type `i32` on thread `unnamed-ID` at ALLOC. (2) just happened here
    |
 help: and (1) occurred earlier here
   --> $DIR/retag_data_race_protected_read.rs:LL:CC