Remove print!/println! code from mmtk-core (#655)

This pull request removes `print!` and `println!` from our code base. `print!/println!` are replaced with logging lines, `write!`, or `eprint!/eprintln!` if we really need to report errors. This closes #654.

Author: qinsoon

src/plan/nogc/global.rs

@@ -80,7 +80,7 @@ impl<VM: VMBinding> Plan for NoGC<VM> {
     }
 
     fn handle_user_collection_request(&self, _tls: VMMutatorThread, _force: bool) {
-        println!("Warning: User attempted a collection request, but it is not supported in NoGC. The request is ignored.");
+        warn!("User attempted a collection request, but it is not supported in NoGC. The request is ignored.");
     }
 }
 

src/policy/space.rs

@@ -608,51 +608,6 @@ pub trait Space<VM: VMBinding>: 'static + SFT + Sync + Downcast {
         panic!("A copying space should override this method")
     }
 
-    fn print_vm_map(&self) {
-        let common = self.common();
-        print!("{} ", common.name);
-        if common.immortal {
-            print!("I");
-        } else {
-            print!(" ");
-        }
-        if common.movable {
-            print!(" ");
-        } else {
-            print!("N");
-        }
-        print!(" ");
-        if common.contiguous {
-            print!("{}->{}", common.start, common.start + common.extent - 1);
-            match common.vmrequest {
-                VMRequest::Extent { extent, .. } => {
-                    print!(" E {}", extent);
-                }
-                VMRequest::Fraction { frac, .. } => {
-                    print!(" F {}", frac);
-                }
-                _ => {}
-            }
-        } else {
-            let mut a = self
-                .get_page_resource()
-                .common()
-                .get_head_discontiguous_region();
-            while !a.is_zero() {
-                print!(
-                    "{}->{}",
-                    a,
-                    a + self.common().vm_map().get_contiguous_region_size(a) - 1
-                );
-                a = self.common().vm_map().get_next_contiguous_region(a);
-                if !a.is_zero() {
-                    print!(" ");
-                }
-            }
-        }
-        println!();
-    }
-
     /// Ensure that the current space's metadata context does not have any issues.
     /// Panics with a suitable message if any issue is detected.
     /// It also initialises the sanity maps which will then be used if the `extreme_assertions` feature is active.
@@ -668,6 +623,66 @@ pub trait Space<VM: VMBinding>: 'static + SFT + Sync + Downcast {
     }
 }
 
+/// Print the VM map for a space.
+/// Space needs to be object-safe, so it cannot have methods that use extra generic type paramters. So this method is placed outside the Space trait.
+/// This method can be invoked on a &dyn Space (space.as_space() will return &dyn Space).
+#[allow(unused)]
+pub(crate) fn print_vm_map<VM: VMBinding>(
+    space: &dyn Space<VM>,
+    out: &mut impl std::fmt::Write,
+) -> Result<(), std::fmt::Error> {
+    let common = space.common();
+    write!(out, "{} ", common.name)?;
+    if common.immortal {
+        write!(out, "I")?;
+    } else {
+        write!(out, " ")?;
+    }
+    if common.movable {
+        write!(out, " ")?;
+    } else {
+        write!(out, "N")?;
+    }
+    write!(out, " ")?;
+    if common.contiguous {
+        write!(
+            out,
+            "{}->{}",
+            common.start,
+            common.start + common.extent - 1
+        )?;
+        match common.vmrequest {
+            VMRequest::Extent { extent, .. } => {
+                write!(out, " E {}", extent)?;
+            }
+            VMRequest::Fraction { frac, .. } => {
+                write!(out, " F {}", frac)?;
+            }
+            _ => {}
+        }
+    } else {
+        let mut a = space
+            .get_page_resource()
+            .common()
+            .get_head_discontiguous_region();
+        while !a.is_zero() {
+            write!(
+                out,
+                "{}->{}",
+                a,
+                a + space.common().vm_map().get_contiguous_region_size(a) - 1
+            )?;
+            a = space.common().vm_map().get_next_contiguous_region(a);
+            if !a.is_zero() {
+                write!(out, " ")?;
+            }
+        }
+    }
+    writeln!(out)?;
+
+    Ok(())
+}
+
 impl_downcast!(Space<VM> where VM: VMBinding);
 
 pub struct CommonSpace<VM: VMBinding> {
@@ -713,9 +728,6 @@ pub struct SpaceOptions {
     pub side_metadata_specs: SideMetadataContext,
 }
 
-/// Print debug info for SFT. Should be false when committed.
-const DEBUG_SPACE: bool = cfg!(debug_assertions) && false;
-
 impl<VM: VMBinding> CommonSpace<VM> {
     pub fn new(
         opt: SpaceOptions,
@@ -805,15 +817,13 @@ impl<VM: VMBinding> CommonSpace<VM> {
             panic!("failed to mmap meta memory");
         }
 
-        if DEBUG_SPACE {
-            println!(
-                "Created space {} [{}, {}) for {} bytes",
-                rtn.name,
-                start,
-                start + extent,
-                extent
-            );
-        }
+        debug!(
+            "Created space {} [{}, {}) for {} bytes",
+            rtn.name,
+            start,
+            start + extent,
+            extent
+        );
 
         rtn
     }

src/util/sanity/memory_scan.rs

@@ -1,6 +1,9 @@
 use crate::util::Address;
 use crate::util::ObjectReference;
 
+// This is legacy code, and no one is using this. Using gdb can achieve the same thing for debugging.
+// The JikesRVM binding still declares this method and we need to remove it from JikesRVM.
+#[deprecated]
 pub fn scan_region() {
     loop {
         let mut buf = String::new();

src/util/statistics/stats.rs

@@ -162,7 +162,7 @@ impl Stats {
             }
             self.shared.increment_phase();
         } else if !self.exceeded_phase_limit.load(Ordering::SeqCst) {
-            println!("Warning: number of GC phases exceeds MAX_PHASES");
+            eprintln!("Warning: number of GC phases exceeds MAX_PHASES");
             self.exceeded_phase_limit.store(true, Ordering::SeqCst);
         }
     }
@@ -178,7 +178,7 @@ impl Stats {
             }
             self.shared.increment_phase();
         } else if !self.exceeded_phase_limit.load(Ordering::SeqCst) {
-            println!("Warning: number of GC phases exceeds MAX_PHASES");
+            eprintln!("Warning: number of GC phases exceeds MAX_PHASES");
             self.exceeded_phase_limit.store(true, Ordering::SeqCst);
         }
     }
@@ -232,9 +232,7 @@ impl Stats {
     pub fn start_all(&self) {
         let counters = self.counters.lock().unwrap();
         if self.get_gathering_stats() {
-            println!("Error: calling Stats.startAll() while stats running");
-            println!("       verbosity > 0 and the harness mechanism may be conflicting");
-            debug_assert!(false);
+            panic!("calling Stats.startAll() while stats running");
         }
         self.shared.set_gathering_stats(true);