fix: have Instance as Cache set key

close #42
close #40

Author: zjp-CN

src/functions/cache.rs

@@ -6,10 +6,7 @@ use super::utils::{SourceCode, source_code_with};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_middle::ty::TyCtxt;
 use rustc_span::source_map::{SourceMap, get_source_map};
-use stable_mir::{
-    CrateDef, DefId,
-    mir::{Body, mono::Instance},
-};
+use stable_mir::mir::{Body, mono::Instance};
 use std::{cell::RefCell, cmp::Ordering, sync::Arc};
 
 thread_local! {
@@ -45,39 +42,20 @@ pub fn get_source_code(inst: &Instance) -> Option<SourceCode> {
     get_cache_func(inst, |cf| cf.src.clone())
 }
 
-/// NOTE: this function doesn't auto insert SourceCode if Instance isn't handled.
-pub fn share_same_source_code(a: &Instance, b: &Instance) -> bool {
-    get_cache(|cache| {
-        let defid_a = a.def.def_id();
-        let defid_b = b.def.def_id();
-        let src_a = cache.set.get(&defid_a);
-        let src_b = cache.set.get(&defid_b);
-        match (src_a, src_b) {
-            (Some(cache_a), Some(cache_b)) => match (cache_a, cache_b) {
-                (Some(func_a), Some(func_b)) => func_a.src == func_b.src,
-                (None, None) => defid_a == defid_b,
-                (None, Some(_)) => false,
-                (Some(_), None) => false,
-            },
-            (None, None) => defid_a == defid_b,
-            (None, Some(_)) => false,
-            (Some(_), None) => false,
-        }
-    })
-}
-
 pub fn cmp_callees(a: &Instance, b: &Instance) -> Ordering {
     get_cache(|cache| {
         cache.get_or_insert(a);
         cache.get_or_insert(b);
-        let func_a = cache.set.get(&a.def.def_id()).unwrap().as_ref().map(|f| &f.src);
-        let func_b = cache.set.get(&b.def.def_id()).unwrap().as_ref().map(|f| &f.src);
+        let func_a = cache.set.get(a).unwrap().as_ref().map(|f| &f.src);
+        let func_b = cache.set.get(b).unwrap().as_ref().map(|f| &f.src);
         func_a.cmp(&func_b)
     })
 }
 
 struct Cache {
-    set: FxHashMap<DefId, Option<CacheFunction>>,
+    /// The reason to have Instance as the key is
+    /// https://github.com/os-checker/distributed-verification/issues/42
+    set: FxHashMap<Instance, Option<CacheFunction>>,
     rustc: Option<RustcCxt>,
     path_prefixes: PathPrefixes,
 }
@@ -91,7 +69,7 @@ impl Cache {
 
     fn get_or_insert(&mut self, inst: &Instance) -> Option<&CacheFunction> {
         self.set
-            .entry(inst.def.def_id())
+            .entry(*inst)
             .or_insert_with(|| {
                 let body = inst.body()?;
                 let rustc = self.rustc.as_ref()?;

src/functions/mod.rs

@@ -1,8 +1,8 @@
-use indexmap::{IndexMap, IndexSet};
+use indexmap::IndexSet;
 use kani::{CallGraph, KANI_TOOL_ATTRS, collect_reachable_items};
 use rustc_middle::ty::TyCtxt;
 use stable_mir::{
-    CrateDef, DefId,
+    CrateDef,
     crate_def::Attribute,
     mir::mono::{Instance, MonoItem},
 };
@@ -53,7 +53,7 @@ pub struct Function {
 
     /// Recursive fnction calls inside the body.
     /// The elements are sorted by file path and fn source code to keep hash value stable.
-    callees: Vec<Instance>,
+    callees: IndexSet<Instance>,
 }
 
 impl Function {
@@ -75,11 +75,7 @@ impl Function {
 
         let mut callees = IndexSet::new();
         callgraph.recursive_callees(item, &mut callees);
-
-        // Multiple instances may share the same defid (or rather SourceCode), so deduplicate them.
-        let mut callees: Vec<_> = callees.into_iter().collect();
         callees.sort_by(cache::cmp_callees);
-        callees.dedup_by(|a, b| cache::share_same_source_code(a, b));
 
         let this = Function { instance, attrs, callees };
         filter(&this).then_some(this)

tests/compare.rs

@@ -1,7 +1,7 @@
 use std::fs::{copy, remove_file};
 
 mod utils;
-use utils::*;
+use utils::{assert_eq, *};
 
 fn get(text: &str, start: &str) -> SerFunction {
     let json = &text[text.find("[\n").unwrap()..];
@@ -30,14 +30,14 @@ fn compare(tmp: &str, v_file: &[&str], f: &str) {
     // the hash value must be the same.
     for i in 0..len - 1 {
         for j in 1..len {
-            // assert_eq!(
-            //     v_func[i].hash, v_func[j].hash,
-            //     "Hash values of {f:?} are not equal: {} ≠ {}",
-            //     v_file[i], v_file[j]
-            // );
-            if v_func[i].hash == v_func[j].hash {
-                println!("Hash values of {f:?} are not equal: {} ≠ {}", v_file[i], v_file[j]);
-            }
+            assert_eq!(
+                v_func[i].hash, v_func[j].hash,
+                "Hash values of {f:?} are not equal: {} ≠ {}",
+                v_file[i], v_file[j]
+            );
+            // if v_func[i].hash == v_func[j].hash {
+            //     println!("Hash values of {f:?} are not equal: {} ≠ {}", v_file[i], v_file[j]);
+            // }
         }
     }
 }