-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[solidity,llvm-context] Improve support for debugging the compiler #32
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just some nitpicks
crates/solidity/src/process/mod.rs
Outdated
.expect("Error reading from input file"); | ||
} | ||
None => { | ||
stdin.read_to_end(&mut buffer).expect("Stdin reading error"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above :)
crates/solidity/src/process/mod.rs
Outdated
match input_file { | ||
Some(ins) => { | ||
ins.read_to_end(&mut buffer) | ||
.expect("Error reading from input file"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: We usually unwrap results in case we are guaranteed to never panic (the rust compiler can not now) and use expect(...)
to explain the reason as to why the unwrap would never panic.
But since the this function does already return a anhow::Result
it seems like here we can be just bubble up the error instead?
crates/solidity/src/process/mod.rs
Outdated
@@ -74,6 +82,11 @@ pub fn call(input: Input) -> anyhow::Result<Output> { | |||
anyhow::anyhow!("{:?} subprocess spawning error: {:?}", executable, error) | |||
})?; | |||
|
|||
#[cfg(debug_assertions)] | |||
if let Some(dbg_config) = &input.debug_config { | |||
let _ = dbg_config.dump_stage_output(&input.contract.path, Some("stage"), &input_json); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is silently swallowing the error here intended? Intuitively should either bubble up or panic here?
e0adf97
to
e2271fd
Compare
crates/solidity/src/process/mod.rs
Outdated
executable, | ||
error, | ||
) | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
}); | |
})?; |
e2271fd
to
2e77b70
Compare
crates/solidity/src/process/mod.rs
Outdated
.read_to_end(&mut buffer) | ||
.map_err(|error| -> anyhow::Result<()> { | ||
anyhow::bail!("Failed to read recursive process input file: {:?}", error); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
}); | |
})?; |
crates/solidity/src/process/mod.rs
Outdated
"Failed to read recursive process input from stdin: {:?}", | ||
error | ||
); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
}); | |
})?; |
Add option --recursive-process-input <filename> for use with --recursive-process to specify the name of a file to use instead of reading from stdin. If --debug-output-dir is set, dump the file passed to the recursive invocation of the compiler as a JSON file suitable for use with --recursive-process-input. These changes are intended to support debugging the compiler and are only available with DEBUG builds.
2e77b70
to
b29381a
Compare
Add option --recursive-process-input for use with --recursive-process to specify the name of a file to use instead of reading from stdin.
If --debug-output-dir is set, dump the file passed to the recursive invocation of the compiler as a JSON file suitable for use with --recursive-process-input.
These changes are intended to support debugging the compiler and are only available with DEBUG builds.