Skip to content

Commit

Permalink
Remove spirv result from Error::display (#20)
Browse files Browse the repository at this point in the history
* Checkpoint fix for #19

* Ignore the spirv error code when in Error::display

This is due to the lossiness of running the spirv binaries, while
needing (more) consistent error output between compiled and tool.
  • Loading branch information
Jake-Shadle authored Mar 25, 2021
1 parent e6527cf commit b0b38a2
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 9 deletions.
5 changes: 4 additions & 1 deletion src/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ impl From<CmdError> for crate::error::Error {
);

Self {
inner: SpirvResult::InternalError, // this isn't really correct
// this isn't really correct, but the spirv binaries don't
// provide the error code in any meaningful way, either by the
// status code of the binary, or in diagnostic output
inner: SpirvResult::InternalError,
diagnostic: Some(diagnostic),
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match &self.diagnostic {
Some(diag) => f.write_fmt(format_args!(
"{}:{}:{} - {}",
self.inner, diag.line, diag.column, diag.message
"error:{}:{} - {}",
diag.line, diag.column, diag.message
)),
None => f.write_fmt(format_args!("{}", self.inner)),
None => f.write_str("an unknown error occurred"),
}
}
}
Expand Down
12 changes: 7 additions & 5 deletions tests/val.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,16 @@ fn validate_tool(_input: &[u8]) -> Option<Result<(), spirv_tools::Error>> {

#[test]
fn gets_error_message() {
let cexpected_msg = "invalid cfg:0:0 - Loop header 6[%loop_header] is targeted by 2 back-edge blocks but the standard requires exactly one\n %loop_header = OpLabel\n";
let texpected_msg = "internal error:0:0 - Loop header 6[%loop_header] is targeted by 2 back-edge blocks but the standard requires exactly one";
let cexpected_msg = "error:0:0 - Loop header 6[%loop_header] is targeted by 2 back-edge blocks but the standard requires exactly one\n %loop_header = OpLabel\n";
let texpected_msg = "error:0:0 - Loop header 6[%loop_header] is targeted by 2 back-edge blocks but the standard requires exactly one";
match (validate_compiled(SPIRV_BIN), validate_tool(SPIRV_BIN)) {
(Some(resc), Some(rest)) => {
// assert_eq!(resc, rest);
let cstr = resc.unwrap_err().to_string();
let tstr = rest.unwrap_err().to_string();
assert_eq!(&cstr[..111], &tstr[..111]);

assert_eq!(resc.unwrap_err().to_string(), cexpected_msg);
assert_eq!(rest.unwrap_err().to_string(), texpected_msg);
assert_eq!(cstr, cexpected_msg);
assert_eq!(tstr, texpected_msg);
}
(Some(resc), None) => {
assert_eq!(resc.unwrap_err().to_string(), cexpected_msg);
Expand Down

0 comments on commit b0b38a2

Please sign in to comment.