Remove loan_(read|write)_refined fields from PermissionsData. Revert stepper changes to use the seperate fields so that the snapshots are easier to reason through.

Author: gavinleroy

crates/aquascope/src/analysis/mod.rs

@@ -57,8 +57,41 @@ pub struct LoanKey(pub u32);
 )]
 #[ts(export)]
 pub enum LoanRefined<T> {
-  RefineOnlyWrite { write_key: T },
-  RefineReadAndWrite { write_key: T },
+  None,
+  Read { key: T },
+  Write { key: T },
+}
+
+impl<T> LoanRefined<T> {
+  pub fn as_read_refinement(self) -> Option<T> {
+    match self {
+      Self::Read { key } => Some(key),
+      _ => None,
+    }
+  }
+
+  pub fn as_write_refinement(self) -> Option<T> {
+    match self {
+      Self::Read { key } | Self::Write { key } => Some(key),
+      _ => None,
+    }
+  }
+
+  pub fn not_refined(&self) -> bool {
+    matches!(self, LoanRefined::None)
+  }
+
+  pub fn is_refined(&self) -> bool {
+    !self.not_refined()
+  }
+
+  pub fn is_read_refined(&self) -> bool {
+    matches!(self, LoanRefined::Read { .. })
+  }
+
+  pub fn is_write_refined(&self) -> bool {
+    self.is_read_refined() || matches!(self, LoanRefined::Write { .. })
+  }
 }
 
 #[derive(

crates/aquascope/src/analysis/permissions/context.rs

@@ -300,9 +300,7 @@ impl<'tcx> PermissionsCtxt<'tcx> {
       type_copyable: ty.is_copyable(self.tcx, self.typing_env),
       path_moved: None,
       path_uninitialized: false,
-      loan_read_refined: None,
-      loan_write_refined: None,
-      loan_refined: None,
+      loan_refined: LoanRefined::None,
       loan_drop_refined: None,
     }
   }
@@ -383,20 +381,16 @@ impl<'tcx> PermissionsCtxt<'tcx> {
     let loan_write_refined: Option<LoanKey> =
       loan_write_refined.get(path).map(Into::<LoanKey>::into);
 
-    let loan_refined: Option<LoanRefined<LoanKey>> = match (
+    let loan_refined: LoanRefined<LoanKey> = match (
       loan_read_refined,
       loan_write_refined,
     ) {
-      (Some(_read_key), Some(write_key)) => {
-        Some(LoanRefined::RefineReadAndWrite { write_key })
-      }
-      (None, Some(write_key)) => {
-        Some(LoanRefined::RefineOnlyWrite { write_key })
-      }
-      (Some(_read_key), None) => {
+      (Some(..), Some(key)) => LoanRefined::Read { key },
+      (None, Some(key)) => LoanRefined::Write { key },
+      (Some(..), None) => {
         unreachable!("If read permissions are lost at a point, write permissions are also lost.")
       }
-      (None, None) => None,
+      (None, None) => LoanRefined::None,
     };
     let loan_drop_refined: Option<LoanKey> =
       loan_drop_refined.get(path).map(Into::<LoanKey>::into);
@@ -408,8 +402,6 @@ impl<'tcx> PermissionsCtxt<'tcx> {
       is_live,
       path_uninitialized,
       path_moved,
-      loan_read_refined,
-      loan_write_refined,
       loan_refined,
       loan_drop_refined,
     }
@@ -434,9 +426,7 @@ impl<'tcx> PermissionsCtxt<'tcx> {
           type_copyable: false,
           path_moved: None,
           path_uninitialized: false,
-          loan_read_refined: None,
-          loan_write_refined: None,
-          loan_refined: None,
+          loan_refined: LoanRefined::None,
           loan_drop_refined: None,
         })
       })

crates/aquascope/src/analysis/permissions/mod.rs

@@ -102,17 +102,9 @@ pub struct PermissionsData {
   #[serde(skip_serializing_if = "Option::is_none")]
   pub path_moved: Option<MoveKey>,
 
-  #[serde(skip_serializing_if = "Option::is_none")]
-  /// Is a live loan removing `read` permissions?
-  pub loan_read_refined: Option<LoanKey>,
-
-  #[serde(skip_serializing_if = "Option::is_none")]
-  /// Is a live loan removing `write` permissions?
-  pub loan_write_refined: Option<LoanKey>,
-
-  #[serde(skip_serializing_if = "Option::is_none")]
+  #[serde(skip_serializing_if = "LoanRefined::not_refined")]
   /// Is a live loan removing `read` or `write` permissions?
-  pub loan_refined: Option<LoanRefined<LoanKey>>,
+  pub loan_refined: LoanRefined<LoanKey>,
 
   #[serde(skip_serializing_if = "Option::is_none")]
   /// Is a live loan removing `drop` permissions?
@@ -147,9 +139,9 @@ impl PermissionsData {
   /// - no drop-refining loan exists at this point.
   pub fn permissions_ignore_liveness(&self) -> Permissions {
     let mem_uninit = self.path_moved.is_some() || self.path_uninitialized;
-    let read = !mem_uninit && self.loan_read_refined.is_none();
+    let read = !mem_uninit && !self.loan_refined.is_read_refined();
     let write =
-      self.type_writeable && read && self.loan_write_refined.is_none();
+      self.type_writeable && read && !self.loan_refined.is_write_refined();
     let drop = self.type_droppable && read && self.loan_drop_refined.is_none();
     Permissions { read, write, drop }
   }

crates/aquascope/src/analysis/stepper/mod.rs

@@ -17,7 +17,6 @@ use rustc_utils::source_map::range::CharRange;
 use serde::{Deserialize, Serialize};
 use ts_rs::TS;
 
-use super::LoanRefined;
 use crate::analysis::{
   permissions::{
     Permissions, PermissionsCtxt, PermissionsData, PermissionsDomain,
@@ -106,6 +105,18 @@ pub enum ValueStep<A: Stepable> {
 }
 
 impl<A: Stepable> ValueStep<A> {
+  fn high(value: A) -> Self {
+    Self::High { value }
+  }
+
+  fn none() -> Self {
+    Self::None { value: None }
+  }
+
+  fn already(value: A) -> Self {
+    Self::None { value: Some(value) }
+  }
+
   // TODO: this is a loose surface-level notion of symmetry.
   fn is_symmetric_diff(&self, rhs: &Self) -> bool {
     matches!(
@@ -168,9 +179,8 @@ pub struct PermissionsDataDiff {
   pub type_writeable: ValueStep<bool>,
   pub path_moved: ValueStep<MoveKey>,
   pub path_uninitialized: ValueStep<bool>,
-  pub loan_read_refined: ValueStep<LoanKey>, // ValueStep<LoanRefined<LoanKey>> where LoanRefined is ReadOnly or ReadPlusWrite
+  pub loan_read_refined: ValueStep<LoanKey>,
   pub loan_write_refined: ValueStep<LoanKey>,
-  pub loan_refined: ValueStep<LoanRefined<LoanKey>>,
   pub loan_drop_refined: ValueStep<LoanKey>,
   pub permissions: PermissionsDiff,
 }
@@ -183,8 +193,16 @@ impl std::fmt::Debug for PermissionsDataDiff {
     writeln!(f, "    type_droppable:     {:?}", self.type_droppable)?;
     writeln!(f, "    type_writeable:     {:?}", self.type_writeable)?;
     writeln!(f, "    path_moved:         {:?}", self.path_moved)?;
-    writeln!(f, "    loan_write_refined: {:?}", self.loan_write_refined)?;
-    writeln!(f, "    loan_refined: {:?}", self.loan_refined)?;
+    writeln!(
+      f,
+      "    loan_read_refined:       {:?}",
+      self.loan_read_refined
+    )?;
+    writeln!(
+      f,
+      "    loan_write_refined:       {:?}",
+      self.loan_write_refined
+    )?;
     writeln!(f, "    loan_drop_refined:  {:?}", self.loan_drop_refined)?;
     Ok(())
   }
@@ -211,9 +229,9 @@ impl Difference for bool {
     if *self && !rhs {
       ValueStep::Low
     } else if !*self && rhs {
-      ValueStep::High { value: true }
+      ValueStep::high(true)
     } else {
-      ValueStep::None { value: Some(*self) }
+      ValueStep::already(*self)
     }
   }
 }
@@ -229,9 +247,9 @@ impl<A: Stepable> Difference for Option<A> {
 
   fn diff(&self, rhs: Option<A>) -> Self::Diff {
     match (self, rhs) {
-      (None, None) => ValueStep::None { value: None },
+      (None, None) => ValueStep::none(),
       (Some(_), None) => ValueStep::Low,
-      (None, Some(value)) => ValueStep::High { value },
+      (None, Some(value)) => ValueStep::high(value),
       (Some(v0), Some(v1)) => {
         if *v0 != v1 {
           log::warn!(
@@ -264,9 +282,14 @@ impl Difference for PermissionsData {
       is_live: self.is_live.diff(rhs.is_live),
       type_droppable: self.type_droppable.diff(rhs.type_droppable),
       type_writeable: self.type_writeable.diff(rhs.type_writeable),
-      loan_read_refined: self.loan_read_refined.diff(rhs.loan_read_refined),
-      loan_write_refined: self.loan_write_refined.diff(rhs.loan_write_refined),
-      loan_refined: self.loan_refined.diff(rhs.loan_refined),
+      loan_read_refined: self
+        .loan_refined
+        .as_read_refinement()
+        .diff(rhs.loan_refined.as_read_refinement()),
+      loan_write_refined: self
+        .loan_refined
+        .as_write_refinement()
+        .diff(rhs.loan_refined.as_write_refinement()),
       loan_drop_refined: self.loan_drop_refined.diff(rhs.loan_drop_refined),
       path_moved: self.path_moved.diff(rhs.path_moved),
       path_uninitialized: self.path_uninitialized.diff(rhs.path_uninitialized),

crates/aquascope/tests/snapshots/boundaries__main@assignop_path_resolution.test.snap

@@ -70,10 +70,9 @@ description: main@assignop_path_resolution.test
     type_copyable: true
     is_live: true
     path_uninitialized: false
-    loan_write_refined: 0
     loan_refined:
-      RefineOnlyWrite:
-        write_key: 0
+      Write:
+        key: 0
     loan_drop_refined: 0
 - location:
     line: 8