Disallow prints in all crates #664
Conversation
|
Do we need to add that line to the main module of every crate or how does it work? |
georgwiese
force-pushed
the
disallow-print
branch
from
b7f25f2
to
39dc216
Compare
Yes, I think so. In the future, this type of stuff can be configured in |
compiler/src/lib.rs
Outdated
| @@ -337,7 +339,7 @@ pub fn inputs_to_query_callback<T: FieldElement>(inputs: Vec<T>) -> impl Fn(&str | |||
| } | |||
| "\"print_char\"" => { | |||
| assert_eq!(items.len(), 2); | |||
| print!("{}", items[1].parse::<u8>().unwrap() as char); | |||
| log::info!("{}", items[1].parse::<u8>().unwrap() as char); | |||
There was a problem hiding this comment.
I'm not really sure about this one. This is not the main ebreak/ecall thin for riscv - do you know where it is used?
There was a problem hiding this comment.
Ah no it actually is the main one!
At some point, we should probably store this into a proper buffer so that it does not interfere with the logging.
Until then, it should probably stay print.
There was a problem hiding this comment.
Searching for print_char, I only found this line, for the RISC-V ebreak instruction!
There was a problem hiding this comment.
OK, I changed it back to print!
georgwiese
force-pushed
the
disallow-print
branch
from
39dc216
to
286051c
Compare
georgwiese
force-pushed
the
disallow-print
branch
from
286051c
to
a28a037
Compare
As far as I can see, there is no way to configure this on a workspace level in stable Rust.
Note also that clippy isn't currently run in the CI. #661 fixes that. Running
cargo clippymanually should catch all instances ofprintln!()andprint!(). I replaced all instances we had in the code withlog::info!().Also note that we currently have another lint error in the code base (also fixed in #661).