`check_unique` refactor
Refactored all the unique and non-unique assert!ions into check_unique, which is much more concise and avoids the ton of repetition there was previously. It also adds more context to the error message when it fails.
One downside I see to this approach is that in some cases it forces the node names in the assertion to be out of order compared to the graph, which makes it slightly harder to match the two up. I think we could get the best of both worlds with a macro along the lines of assert_unique!(pdg, [a, !j, !b1, !b2, !c1, !c2, d2]);, or something like this:
let [a, j, b1, b2, c1, c2, d2] = unique_flags([a, j, b1, b2, c1, c2, d2]);
assert!(a && !j && !b1 && !b2 && !c1 && !c2 && d2);
One downside I see to this approach is that in some cases it forces the node names in the assertion to be out of order compared to the graph, which makes it slightly harder to match the two up. I think we could get the best of both worlds with a macro along the lines of
assert_unique!(pdg, [a, !j, !b1, !b2, !c1, !c2, d2]);, or something like this:let [a, j, b1, b2, c1, c2, d2] = unique_flags([a, j, b1, b2, c1, c2, d2]); assert!(a && !j && !b1 && !b2 && !c1 && !c2 && d2);
See #668. That solves this issue I think by putting the expected unique value where we create the Node.
I think we could get the best of both worlds with a macro along the lines of
assert_unique!(pdg, [a, !j, !b1, !b2, !c1, !c2, d2]);I don't think we should add a macro if we don't need to.
or something like this:
let [a, j, b1, b2, c1, c2, d2] = unique_flags([a, j, b1, b2, c1, c2, d2]); assert!(a && !j && !b1 && !b2 && !c1 && !c2 && d2);
This wouldn't give good error messages, though.
This whole PR is superseded by the approach in #668, right?
This whole PR is superseded by the approach in #668, right?
Yeah, pretty much.