Should not sleep Fixes

crates/dreamchecker/README.md

@@ -62,6 +62,7 @@ be enabled:
 	#define UNLINT(X) SpacemanDMM_unlint(X)
 	#define SHOULD_NOT_OVERRIDE(X) set SpacemanDMM_should_not_override = X
 	#define SHOULD_NOT_SLEEP(X) set SpacemanDMM_should_not_sleep = X
+    #define ALLOWED_TO_SLEEP(X) set SpacemanDMM_allowed_to_sleep = X
 	#define SHOULD_BE_PURE(X) set SpacemanDMM_should_be_pure = X
 	#define PRIVATE_PROC(X) set SpacemanDMM_private_proc = X
 	#define PROTECTED_PROC(X) set SpacemanDMM_protected_proc = X
@@ -75,6 +76,7 @@ be enabled:
 	#define UNLINT(X) X
 	#define SHOULD_NOT_OVERRIDE(X)
 	#define SHOULD_NOT_SLEEP(X)
+    #define ALLOWED_TO_SLEEP(X)
 	#define SHOULD_BE_PURE(X)
 	#define PRIVATE_PROC(X)
 	#define PROTECTED_PROC(X)

crates/dreamchecker/src/lib.rs

@@ -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,66 +641,90 @@ impl<'o> AnalyzeObjectTree<'o> {
     }
 
     pub fn check_proc_call_tree(&mut self) {
-        for (procref, &(_, location)) in self.must_not_sleep.directive.iter() {
+        // prepare for the worst case, avoiding the reallocations _is_ faster and less memory expensive
+        let total_procs = self.objtree.iter_types().flat_map(|type_ref: TypeRef| type_ref.iter_self_procs()).count();
+        let mut visited = HashSet::<ProcRef<'o>>::with_capacity(total_procs);
+        let mut to_visit = VecDeque::<(ProcRef<'o>, CallStack, bool, ProcRef<'o>)>::new();
+        for (index, (procref, &(_, location))) in self.must_not_sleep.directive.iter().enumerate() {
+            if !visited.insert(*procref) {
+                continue;
+            }
+
             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))
                     .with_note(location, "SpacemanDMM_should_not_sleep set here")
                     .with_errortype("must_not_sleep")
                     .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));
+
+            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() {
-                if !visited.insert(nextproc) {
+
+            let procref_type_index = procref.ty().index();
+            while let Some((nextproc, callstack, new_context, parent_proc)) = to_visit.pop_front() {
+                if new_context {
                     continue
                 }
-                if self.waitfor_procs.get(&nextproc).is_some() {
+                if !visited.insert(nextproc) {
                     continue
                 }
-                if self.sleep_exempt.get(nextproc).is_some() {
+                if self.waitfor_procs.contains(&nextproc) {
                     continue
                 }
-                if new_context {
+                if self.sleep_exempt.get(nextproc).is_some() {
                     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 desc = 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
-                        }
-                        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)
-                    }
+                        .register(self.context);
+
+                    // Abort now, we don't want to go unnecessarily deep
+                    continue
+                }
+
+                if nextproc.ty().index() != self.objtree.root().index() && nextproc.name() != "New" {
+                    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));
                     }
                 }
             }
         }
 
+        drop(visited);
+        drop(to_visit);
+
         for (procref, (_, location)) in self.must_be_pure.directive.iter() {
             if let Some(impurevec) = self.impure_procs.get_violators(*procref) {
                 error(procref.get().location, format!("{} does impure operations", procref))
@@ -829,13 +851,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);
+}