Insufficient params test should not rely on panic formatting
Closed this issue · 9 comments
installation
I have a M2 MacBook Air.
git clone https://github.com/cucapra/calyx.git
cargo build
brew install jq
cargo install runt --version 0.4.0 # the latest is 0.4.1 which doesn't worktesting
$ runt -i core
✗ [core] errors:tests/errors/insufficient-params.futil
240 passing / 1 failing / 0 missing / 0 skipped / 0 remainingversion
$ ./target/debug/calyx --version
Calyx compiler version 0.6.1
Library location: /Users/ethan/.calyxCan you run runt -i core -d so that it tells you why the test output didn't match?
I recognize this problem. 🤪
- extern "<ROOT>/calyx/tests/passes/canonical/dummy.sv" {
+ extern "/Users/ethan/Documents/GitHub/calyx_git_clone_original_source/tests/passes/canonical/dummy.sv" {Here's the problem: those emitted extern statements in compiled Calyx code contain absolute paths, which of course differ on everybody's machine. We have a hack in our test harness to work around this by replacing the machine-specific part of the path with the magic string <ROOT>:
Line 12 in 94dd5b0
However, this hack depends on your Calyx checkout's directory name being calyx, verbatim. So if you were to rename calyx_git_clone_original_source to calyx, this would work.
I have never had any brilliant ideas for how to remove this hack (e.g.., to practically have the compiler output relative paths), but ideas are always welcome…
Separately, what you're seeing for the insufficient-params.futil test is a Rust panic:
thread 'main' panicked at 'Failed to add primitive.: Malformed Structure: Invali ...
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
fatal runtime error: failed to initiate panic, error 5
As the error message suggests, it would be great to try running this command manually with backtraces enabled to see what's going wrong!
I have never had any brilliant ideas for how to remove this hack (e.g.., to practically have the compiler output relative paths), but ideas are always welcome…
Likely I don't understand, but if you're trying to deal with relative paths when you don't want to install in some centralized /usr/lib-esque location, can't we do what C/C++ tooling does and have some environment variable akin to CPATH or flag akin to -I/--sysroot?
Separately, what you're seeing for the
insufficient-params.futiltest is a Rust panic:thread 'main' panicked at 'Failed to add primitive.: Malformed Structure: Invali ... note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace fatal runtime error: failed to initiate panic, error 5As the error message suggests, it would be great to try running this command manually with backtraces enabled to see what's going wrong!
✗ [core] errors:tests/errors/insufficient-params.futil
~
1 1│ ---CODE---
2 │- 101
2│+ -1
3 3│ ---STDERR---
4 4│ thread 'main' panicked at 'Failed to add primitive.: Malformed Structure: Invali ...
5 │- note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
5│+ stack backtrace:
6│+ 0: 0x100dabfa8 - std::backtrace_rs::backtrace::libunwind::trace::h65bc41667b946a9b
7│+ at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
8│+ 1: 0x100dabfa8 - std::backtrace_rs::backtrace::trace_unsynchronized::h11e85e00c64d2951
9│+ at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
10│+ 2: 0x100dabfa8 - std::sys_common::backtrace::_print_fmt::hd30f3b7be3b8d9bb
11│+ at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/sys_common/backtrace.rs:65:5
12│+ 3: 0x100dabfa8 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h56d7672c82815b65
13│+ at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/sys_common/backtrace.rs:44:22
14│+ 4: 0x100dc44f4 - core::fmt::rt::Argument::fmt::h6ead82aa10d38e42
15│+ at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/core/src/fmt/rt.rs:138:9
16│+ 5: 0x100dc44f4 - core::fmt::write::h632b3cc8e66b4f04
17│+ at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/core/src/fmt/mod.rs:1094:21
18│+ 6: 0x100da9c44 - std::io::Write::write_fmt::hc28441b249a4971d
19│+ at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/io/mod.rs:1714:15
20│+ 7: 0x100dabe00 - std::sys_common::backtrace::_print::h0d2f8b8a08c5cc48
21│+ at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/sys_common/backtrace.rs:47:5
22│+ 8: 0x100dabe00 - std::sys_common::backtrace::print::h8057ced4b0f9fdc1
23│+ at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/sys_common/backtrace.rs:34:9
24│+ 9: 0x100dad224 - std::panicking::default_hook::{{closure}}::h8157fa8f0f7934b5
25│+ at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/panicking.rs:269:22
26│+ 10: 0x100dacfb4 - std::panicking::default_hook::hdaefe3bfb5fe212c
27│+ at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/panicking.rs:288:9
28│+ fatal runtime error: failed to initiate panic, error 5
6 29│
~You can do runt -n tests/errors/insufficient-params.futil to get the exact command that the test runs. Then you can try running the command and playing around with it to see what's wrong.
This bug is probably not reproducible for us because the test passes on the CI. My best guess is that there is something different about Rust versions.
I ran into something similar while trying to get a new install up and running. You're right about the rust version, looks like panic messages are not stable across rust versions. I'm on 1.75.0. I guess a short-term solution would be to pin a rust version with a rust-toolchain.toml file. A maybe more robust solution is to not panic in the builder so that we can control the stability of the error messages.
✗ [core] errors:tests/errors/insufficient-params.futil
~
2 2│ 101
3 3│ ---STDERR---
4 │- thread 'main' panicked at 'Failed to add primitive.: Malformed Structure: Invalid parameter binding for primitive `std_fp_div_pipe`. Requires 3 parameters but provided with 1.', calyx-ir/src/builder.rs:224:14
4│+ thread 'main' panicked at calyx-ir/src/builder.rs:224:14:
5│+ Failed to add primitive.: Malformed Structure: Invalid parameter binding for primitive `std_fp_div_pipe`. Requires 3 parameters but provided with 1.
5 6│ note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
6 7│
~
Yeah, this has been a consistent problem unfortunately. Not sure what the best solution is here because parameter resolution happens in the AST -> IR conversion step which doesn't bubble up errors. It might be possible to define a version of the method that returns Result.
Yes, indeed! It seems like we can either make that change as @rachitnigam outlines or add a separate AST-level check for these problems (so the subsequent conversion is guaranteed to succeed).