yupferris
commented
4 years ago Note that a Rust feature I was unaware of until now is track_caller, which might be useful for providing better error messages by providing information about the calling function.
A quick test:
use std::panic::Location;
#[track_caller]
fn dude(x: i32) -> i32 {
let caller = Location::caller();
println!("file: {}, line: {}, column: {}", caller.file(), caller.line(), caller.column());
x * 2
}
fn main() {
let x = dude(56);
println!("Hello, world! {}", x);
}So it seems this is useful even outside of a panic context. It's worth exploring this as a way to provide better errors on failure without requiring RUST_BACKTRACE to be set, especially as it's stable as of Rust 1.46.0.
Note also that this could potentially allow us to defer all error checking during graph creation, and then have all error(s) reported at compile/lowering time, perhaps checked by the validation pass that's already in place (but would be extended). I think this makes certain classes of errors a bit harder to check (we need to traverse more of the graph than just the reachable parts like we do currently, which might mean keeping more internal references or something), but it would also mean eliminating all panics from graph creation, which could potentially be really nice. We could also only check for errors in the reachable part of the graph, but I would argue this is generally undesirable, as code that creates unreachable parts of the graph would rot instead of being checked (even though it's ignored during further compilation passes).
jdonszelmann
kaze can only detect certain kinds of errors at certain times. Some examples:
Signals with otherSignals. This is expressed in rust's type system, and is thus detectable/reported during compilation of the rust kaze code.Signals with certainbit_widths with otherSignals of the samebit_widthfor certain operations, eg.x & y. Since aSignal'sbit_widthcannot be described with rust's type system (yet), but checking for this error is trivial, we detect and report this error during execution of the rust kaze code as the graph is being constructed (as part of the&operator impl in this case).Moduleinstance has unconnected inputs, but we must allow unconnected inputs during graph creation and expect them to be resolved before code is generated. In this case, we defer error checking/reporting until codegen time.Further, a somewhat unsatisfying decision that I've made is that certain classes of errors are reported in different ways. Obviously errors that are checked by rust's type checker are reported as rust type errors. However, other types of errors are reported as
panics. The reasoning behind this is to naturally be able to provide where in the rust code the error occurred (via the stack trace), similar to using a non-embedded language. This choice also leads to more readable user/API code, since the types aren't conflated with error handling details that would have to be used on everySignalparameter and return type. It's somewhat indirect though admittedly from a user's perspective, since a stack trace must be used to obtain this error information, as rust doesn't provide any way to get information about a function's caller out-of-the-box (this could potentially be remedied by having macro wrappers for all API entry points that expand to larger calls with additional file/line info but this also gets messy for existingOpimpls which don't allow the API to be extended). This gets a bit messy for codegen as well, where it's natural for the code generator APIs to return aResult(which they actually do currently), but we currently still report those errors aspanics and only useResultto report IO errors. This is arguably consistent with other runtime errors (which alsopanic) and this is why I made this choice, but it's also perfectly reasonable to say that this is inconsistent with its own API which returns anio::Result(though I choseio::Resultspecifically to communicate that only io-related errors are produced this way).One thing we could possibly explore is to use
Result<Signal<..>>for all API entry points instead ofSignaldirectly and have all ops propagate errors in addition to the logic they already do. This would mean that this kind of propagation also needs to be tested for all ops and I think the code would generally be a lot less readable; perhaps there's a good pattern for type aliasing that would help in this regard. This is especially important for higher-order user code that can be used to generate constructs more abstractly which usually already carries additional mental weight, and shouldn't be further complicated by additional type information.Generally this is kindof unsatisfying and probably surprising so at the very least, it should be documented as part of how to use kaze generally and other strategies may need to be investigated for future library versions.