Unreachable code in circular reference check?

[jmm]

When I was doing #12 I started looking at deduplicating the circular reference checks but got bogged down with code style issues. Then I wondered if all of that code is actually reachable. I tried to run the coverage report but it errored out for me. But as demonstrated by this PR, the chunk I'm referring to is not covered; with this change existing tests (and those I add in #12) pass. Is there a situation I haven't thought of where this would be reachable?

[ljharb]

ok, so - I added it initially because I was being careful that both iteration of "value" and "other" had the same checks.

However, I think I see why this change works - because the only reason to have the second iteration in the first place is to ensure that there are no keys in "other" that are missing from "value".

Since all the tests pass, I think this is OK - can you rebase this on the latest master, and I'll bring it in? (bonus points if you can prefix the commit message with "[Refactor]")

[jmm]

However, I think I see why this change works - because the only reason to have the second iteration in the first place is to ensure that there are no keys in "other" that are missing from "value".

Yeah, I can't see a way for the iteration of other to get past that check.

Since all the tests pass, I think this is OK - can you rebase this on the latest master, and I'll bring it in?

Sure, done ✨

[ljharb]

Thanks!

[jmm]

Thanks!