Skip to content

Conversation

@qinsoon
Copy link
Member

@qinsoon qinsoon commented Aug 31, 2022

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

@qinsoon qinsoon marked this pull request as ready for review August 31, 2022 01:04
@qinsoon qinsoon requested a review from wks August 31, 2022 01:04
}

fn print_vm_map(&self) {
fn print_vm_map(&self) -> String {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A more general solution is to write to a &mut impl Write. It can write to a string in this way, and also a file, too, if the user desires. The Write trait still allows us to use the write! or writeln! macros for formatting, like the print! macro in the old code.

Example:

use std::fmt::{Error, Write};

fn print_something(out: &mut impl Write) -> Result<(), Error> {
    writeln!(out, "Hello world!")?;
    writeln!(out, "Number: {}", 42)?;

    Ok(())
}

fn main() {
    let mut s = String::new();
    print_something(&mut s).unwrap();
    println!("{}", s);
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. I changed the method. However, as we cannot have methods with generic type parameters (including impl T) in a object-safe trait (Space), I moved the method outside the trait.

Copy link
Collaborator

@wks wks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@qinsoon qinsoon merged commit 26ff9e1 into mmtk:master Aug 31, 2022
wks added a commit to mmtk/ruby that referenced this pull request Aug 31, 2022
This reverts commit de7fb26.

The upstream fixed the printing issue in:
mmtk/mmtk-core#655
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Don't print to stdout on NoGC GC request output

2 participants