Should not sleep Fixes

crates/dreamchecker/src/lib.rs

@@ -327,7 +327,7 @@ fn run_inner(context: &Context, objtree: &ObjectTree, cli: bool) {
 
     check_var_defs(objtree, context);
 
-    let mut analyzer = AnalyzeObjectTree::new(context, objtree);
+    let mut analyzer = AnalyzeObjectTree::new(context, objtree.clone());
 
     cli_println!("============================================================");
     cli_println!("Gathering proc settings...\n");
@@ -562,7 +562,6 @@ pub struct AnalyzeObjectTree<'o> {
     impure_procs: ViolatingProcs<'o>,
     waitfor_procs: HashSet<ProcRef<'o>>,
 
-    sleeping_overrides: ViolatingOverrides<'o>,
     impure_overrides: ViolatingOverrides<'o>,
 }
 
@@ -588,7 +587,6 @@ impl<'o> AnalyzeObjectTree<'o> {
             sleeping_procs: Default::default(),
             impure_procs: Default::default(),
             waitfor_procs: Default::default(),
-            sleeping_overrides: Default::default(),
             impure_overrides: Default::default(),
         }
     }
@@ -643,6 +641,10 @@ impl<'o> AnalyzeObjectTree<'o> {
     }
 
     pub fn check_proc_call_tree(&mut self) {
+        let mut sleeping_procs_transitive = ViolatingProcs{
+            violators: HashMap::new()
+        };
+
         for (procref, &(_, location)) in self.must_not_sleep.directive.iter() {
             if let Some(sleepvec) = self.sleeping_procs.get_violators(*procref) {
                 error(procref.get().location, format!("{} sets SpacemanDMM_should_not_sleep but calls blocking built-in(s)", procref))
@@ -651,19 +653,26 @@ impl<'o> AnalyzeObjectTree<'o> {
                     .with_blocking_builtins(sleepvec)
                     .register(self.context)
             }
+
             let mut visited = HashSet::<ProcRef<'o>>::new();
-            let mut to_visit = VecDeque::<(ProcRef<'o>, CallStack, bool)>::new();
-            if let Some(procscalled) = self.call_tree.get(procref) {
-                for (proccalled, location, new_context) in procscalled {
-                    let mut callstack = CallStack::default();
-                    callstack.add_step(*proccalled, *location, *new_context);
-                    to_visit.push_back((*proccalled, callstack, *new_context));
+            let mut to_visit = VecDeque::<(ProcRef<'o>, CallStack, bool, ProcRef<'o>)>::new();
+
+            visited.insert(*procref);
+
+            if let Some(calledvec) = self.call_tree.get(procref) {
+                for (proccalled, location, new_context) in calledvec.iter() {
+                    let mut newstack = CallStack::default();
+                    newstack.add_step(*proccalled, *location, *new_context);
+                    to_visit.push_back((*proccalled, newstack, *new_context, *proccalled));
                 }
             }
-            while let Some((nextproc, callstack, new_context)) = to_visit.pop_front() {
+
+            let procref_type_index = procref.ty().index();
+            while let Some((nextproc, callstack, new_context, parent_proc)) = to_visit.pop_front() {
                 if !visited.insert(nextproc) {
                     continue
                 }
+
                 if self.waitfor_procs.get(&nextproc).is_some() {
                     continue
                 }
@@ -673,31 +682,62 @@ impl<'o> AnalyzeObjectTree<'o> {
                 if new_context {
                     continue
                 }
-                if let Some(sleepvec) = self.sleeping_procs.get_violators(nextproc) {
-                    error(procref.get().location, format!("{} sets SpacemanDMM_should_not_sleep but calls blocking proc {}", procref, nextproc))
+
+                let parent_proc_type_index = parent_proc.ty().index();
+                let next_proc_type_index = nextproc.ty().index();
+
+                let proc_is_on_same_type_as_setting = next_proc_type_index == procref_type_index;
+                let proc_is_override = next_proc_type_index != parent_proc_type_index;
+
+                let mut sleepvec_option = sleeping_procs_transitive.get_violators(nextproc);
+                let transitive = sleepvec_option.is_some();
+                if !transitive {
+                    sleepvec_option = self.sleeping_procs.get_violators(nextproc);
+                }
+
+                if let Some(sleepvec) = sleepvec_option {
+                    let desc = if transitive {
+                        format!("{} calls {} which transitively sleeps", procref, nextproc)
+                    } else if proc_is_on_same_type_as_setting && proc_is_override {
+                        format!("{} sets SpacemanDMM_should_not_sleep but has override child proc that sleeps {}", procref, nextproc)
+                    } else if proc_is_override {
+                        format!("{} calls {} which has override child proc that sleeps {}", procref, parent_proc, nextproc)
+                    } else {
+                        format!("{} sets SpacemanDMM_should_not_sleep but calls blocking proc {}", procref, nextproc)
+                    };
+
+                    error(procref.get().location, desc)
                         .with_note(location, "SpacemanDMM_should_not_sleep set here")
                         .with_errortype("must_not_sleep")
                         .with_callstack(&callstack)
                         .with_blocking_builtins(sleepvec)
-                        .register(self.context)
-                } else if let Some(overridesleep) = self.sleeping_overrides.get_override_violators(nextproc) {
-                    for child_violator in overridesleep {
-                        if procref.ty().is_subtype_of(&nextproc.ty()) && !child_violator.ty().is_subtype_of(&procref.ty()) {
-                            continue
+                        .register(self.context);
+
+                    if callstack.call_stack.len() > 1 {
+                        let working_sleepvec = sleepvec.clone();
+                        for (builtin, location) in working_sleepvec {
+                            let mut working_callstack = callstack.call_stack.clone();
+                            working_callstack.pop_back();
+                            for (calling_proc, _calling_location, _new_context) in working_callstack {
+                                sleeping_procs_transitive.insert_violator(calling_proc, &builtin, location);
+                            }
                         }
-                        error(procref.get().location, format!("{} calls {} which has override child proc that sleeps {}", procref, nextproc, child_violator))
-                            .with_note(location, "SpacemanDMM_should_not_sleep set here")
-                            .with_errortype("must_not_sleep")
-                            .with_callstack(&callstack)
-                            .with_blocking_builtins(self.sleeping_procs.get_violators(*child_violator).unwrap())
-                            .register(self.context)
                     }
                 }
+
+                // no more recursion if we hit a transitive sleeper, we've already been there
+                if transitive {
+                    continue;
+                }
+
+                nextproc.recurse_children(&mut |child_proc|
+                    to_visit.push_back((child_proc, callstack.clone(), false, nextproc)));
+
                 if let Some(calledvec) = self.call_tree.get(&nextproc) {
                     for (proccalled, location, new_context) in calledvec.iter() {
                         let mut newstack = callstack.clone();
                         newstack.add_step(*proccalled, *location, *new_context);
-                        to_visit.push_back((*proccalled, newstack, *new_context));
+                        to_visit.push_back((*proccalled, newstack, *new_context, *proccalled));
                     }
                 }
             }
@@ -829,13 +869,6 @@ impl<'o> AnalyzeObjectTree<'o> {
         if proc.name() == "New" { // New() propogates via ..() and causes weirdness
             return;
         }
-        if self.sleeping_procs.get_violators(proc).is_some() {
-            let mut next = proc.parent_proc();
-            while let Some(current) = next {
-                self.sleeping_overrides.insert_override(current, proc);
-                next = current.parent_proc();
-            }
-        }
         if self.impure_procs.get_violators(proc).is_some() {
             let mut next = proc.parent_proc();
             while let Some(current) = next {

crates/dreamchecker/src/test_helpers.rs

@@ -26,23 +26,36 @@ pub fn check_errors_match<S: Into<Cow<'static, str>>>(buffer: S, errorlist: &[(u
     let errors = context.errors();
     let mut iter = errors.iter();
     for (line, column, desc) in errorlist {
-        let nexterror = iter.next().unwrap();
-        if nexterror.location().line != *line
-            || nexterror.location().column != *column
-            || nexterror.description() != *desc
-        {
-            panic!(
-                "possible feature regression in dreamchecker, expected {}:{}:{}, found {}:{}:{}",
-                *line,
-                *column,
-                *desc,
-                nexterror.location().line,
-                nexterror.location().column,
-                nexterror.description()
-            );
+        let nexterror_option = iter.next();
+        match nexterror_option {
+            Some(nexterror) => {
+                if nexterror.location().line != *line
+                    || nexterror.location().column != *column
+                    || nexterror.description() != *desc
+                {
+                    panic!(
+                        "possible feature regression in dreamchecker, expected {}:{}:{}, found {}:{}:{}",
+                        *line,
+                        *column,
+                        *desc,
+                        nexterror.location().line,
+                        nexterror.location().column,
+                        nexterror.description()
+                    );
+                }
+            },
+            None => {
+                panic!(
+                    "possible feature regression in dreamchecker, expected {}:{}:{}, found no additional errors!",
+                    *line,
+                    *column,
+                    *desc
+                );
+            }
         }
     }
-    if iter.next().is_some() {
-        panic!("found more errors than was expected");
+    if let Some(error) = iter.next() {
+        let error_loc = error.location();
+        panic!("found more errors than was expected: {}:{}:{}", error_loc.line, error_loc.column, error.description());
     }
 }

crates/dreamchecker/tests/pure_tests.rs

@@ -0,0 +1,48 @@
+extern crate dreamchecker as dc;
+
+use dc::test_helpers::check_errors_match;
+
+const PURE_ERRORS: &[(u32, u16, &str)] = &[
+    (12, 16, "/mob/proc/test2 sets SpacemanDMM_should_be_pure but calls a /proc/impure that does impure operations"),
+];
+
+#[test]
+fn pure() {
+    let code = r##"
+/proc/pure()
+    return 1
+/proc/impure()
+    world << "foo"
+/proc/foo()
+    pure()
+/proc/bar()
+    impure()
+/mob/proc/test()
+    set SpacemanDMM_should_be_pure = TRUE
+    return foo()
+/mob/proc/test2()
+    set SpacemanDMM_should_be_pure = TRUE
+    bar()
+"##.trim();
+    check_errors_match(code, PURE_ERRORS);
+}
+
+// these tests are separate because the ordering the errors are reported in isn't determinate and I CBF figuring out why -spookydonut Jan 2020
+// TODO: find out why
+const PURE2_ERRORS: &[(u32, u16, &str)] = &[
+    (5, 5, "call to pure proc test discards return value"),
+];
+
+#[test]
+fn pure2() {
+    let code = r##"
+/mob/proc/test()
+    set SpacemanDMM_should_be_pure = TRUE
+    return 1
+/mob/proc/test2()
+    test()
+/mob/proc/test3()
+    return test()
+"##.trim();
+    check_errors_match(code, PURE2_ERRORS);
+}