check_unique refactor

[kkysen]

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.

[spernsteiner]

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);

[kkysen]

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.

[aneksteind]

having these two arguments really doesn't make clear that you're checking for non-uniqueness here, in addition to the function name. i suggest having one function for each

[kkysen]

Yeah, that might be better. Might not be worth changing, though, given #668.

[aneksteind]

see comment

[kkysen]

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.

[spernsteiner]

This whole PR is superseded by the approach in #668, right?

[kkysen]

This whole PR is superseded by the approach in #668, right?

Yeah, pretty much.