Simplify pointer implementation

Before this commit, Concordance attempted to determine whether a complex
value was reused in the same location on either side of a comparison. It
did this by detecting value reuse and emitting a Pointer descriptor
instead. This required lookups to be maintained, to map pointers back to
previously seen descriptors. It turned out that maintaining these
lookups is hard, and there were various code paths where Concordance
should have added descriptors to the lookup tables but didn't.
Furthermore, pointer indexes are simple integer increments, which means
that the same value in either side of the comparison may actually have a
different index. This breaks the comparison logic.

avajs/ava#1431 shows some of the issues that
arose from this approach.

This commit simplifies the implementation. When describing, if the same
value is encountered then the first descriptor is emitted. The
serialization logic tracks complex descriptors by their pointer index
and serializes a pointer descriptor if reuse is detected.
Deserialization ensures that the first descriptor is emitted, rather
than the pointer.

Circular references are now tracked for each side of the comparison. A
stack is maintained, and if at any stage in the comparison a circular
reference is detected, then the result of that comparison is only equal
if both sides have the same circular reference. Which is a long way of
saying that some circular references are considered equal, and others
are not. This shouldn't matter much for real-life uses of Concordance,
though it is a breaking change.

README.md

@@ -31,6 +31,9 @@ means Concordance's behavior is consistent, no matter how you use it.
   Symbol properties are compared by identity.
 * `Promise` values are compared by identity only.
 * `Symbol` values are compared by identity only.
+* Recursion stops whenever a circular reference is encountered. If the same
+  cycle is present in the actual and expected values they're considered equal,
+  but they're unequal otherwise.
 
 ### Formatting details
 

lib/Circular.js

@@ -1,11 +1,35 @@
 'use strict'
 
-class Circular extends Set {
+class Circular {
+  constructor () {
+    this.stack = new Map()
+  }
+
   add (descriptor) {
+    if (this.stack.has(descriptor)) throw new Error('Already in stack')
+
     if (descriptor.isItem !== true && descriptor.isMapEntry !== true && descriptor.isProperty !== true) {
-      super.add(descriptor)
+      this.stack.set(descriptor, this.stack.size + 1)
     }
     return this
   }
+
+  delete (descriptor) {
+    if (this.stack.has(descriptor)) {
+      if (this.stack.get(descriptor) !== this.stack.size) throw new Error('Not on top of stack')
+      this.stack.delete(descriptor)
+    }
+    return this
+  }
+
+  has (descriptor) {
+    return this.stack.has(descriptor)
+  }
+
+  get (descriptor) {
+    return this.stack.has(descriptor)
+      ? this.stack.get(descriptor)
+      : 0
+  }
 }
 module.exports = Circular

lib/Registry.js

@@ -1,7 +1,5 @@
 'use strict'
 
-const describePointer = require('./metaDescriptors/pointer').describe
-
 class Registry {
   constructor () {
     this.counter = 0
@@ -13,12 +11,13 @@ class Registry {
   }
 
   get (value) {
-    return this.map.get(value)
+    return this.map.get(value).descriptor
   }
 
-  add (value) {
-    const pointer = this.counter++
-    this.map.set(value, describePointer(pointer))
+  alloc (value) {
+    const index = ++this.counter
+    const pointer = {descriptor: null, index}
+    this.map.set(value, pointer)
     return pointer
   }
 }

lib/compare.js

@@ -23,40 +23,22 @@ function shortcircuitPrimitive (value) {
 }
 
 function compareDescriptors (lhs, rhs) {
-  const circular = new Circular()
-  const lhsLookup = new Map()
-  const rhsLookup = new Map()
+  const lhsCircular = new Circular()
+  const rhsCircular = new Circular()
 
   const lhsStack = []
   const rhsStack = []
   let topIndex = -1
 
   do {
     let result
-
-    if (lhs.isComplex === true) {
-      lhsLookup.set(lhs.pointer, lhs)
-    }
-    if (rhs.isComplex === true) {
-      rhsLookup.set(rhs.pointer, rhs)
-    }
-
-    if (lhs.isPointer === true) {
-      if (rhs.isPointer === true && lhs.compare(rhs) === DEEP_EQUAL) {
-        result = DEEP_EQUAL
-      } else {
-        lhs = lhsLookup.get(lhs.pointer)
-      }
-    }
-    if (rhs.isPointer === true) {
-      rhs = rhsLookup.get(rhs.pointer)
-    }
-
-    if (circular.has(lhs) && circular.has(rhs)) {
+    if (lhsCircular.has(lhs)) {
+      result = lhsCircular.get(lhs) === rhsCircular.get(rhs)
+        ? DEEP_EQUAL
+        : UNEQUAL
+    } else if (rhsCircular.has(rhs)) {
       result = UNEQUAL
-    }
-
-    if (!result) {
+    } else {
       result = lhs.compare(rhs)
     }
 
@@ -76,11 +58,11 @@ function compareDescriptors (lhs, rhs) {
         rhsStack[topIndex].recursor = recursorUtils.unshift(rhsStack[topIndex].recursor, rhs.collectAll())
       }
 
-      circular.add(lhs)
-      circular.add(rhs)
+      lhsCircular.add(lhs)
+      rhsCircular.add(rhs)
 
-      lhsStack.push({ origin: lhs, recursor: lhs.createRecursor() })
-      rhsStack.push({ origin: rhs, recursor: rhs.createRecursor() })
+      lhsStack.push({ subject: lhs, recursor: lhs.createRecursor() })
+      rhsStack.push({ subject: rhs, recursor: rhs.createRecursor() })
       topIndex++
     }
 
@@ -94,8 +76,8 @@ function compareDescriptors (lhs, rhs) {
       if (lhs === null && rhs === null) {
         const lhsRecord = lhsStack.pop()
         const rhsRecord = rhsStack.pop()
-        circular.delete(lhsRecord.origin)
-        circular.delete(rhsRecord.origin)
+        lhsCircular.delete(lhsRecord.subject)
+        rhsCircular.delete(rhsRecord.subject)
         topIndex--
       } else {
         return false

lib/describe.js

@@ -99,7 +99,7 @@ function describeComplex (value, registry, tryPlugins, describeAny, describeItem
 
   const stringTag = getStringTag(value)
   const ctor = getCtor(stringTag, value)
-  const pointer = registry.add(value)
+  const pointer = registry.alloc(value)
 
   let unboxed
   let describeValue = tryPlugins(value, stringTag, ctor)
@@ -115,17 +115,20 @@ function describeComplex (value, registry, tryPlugins, describeAny, describeItem
       }
     }
   }
-  return describeValue({
+
+  const descriptor = describeValue({
     ctor,
     describeAny,
     describeItem,
     describeMapEntry,
     describeProperty,
-    pointer,
+    pointer: pointer.index,
     stringTag,
     unboxed,
     value
   })
+  pointer.descriptor = descriptor
+  return descriptor
 }
 
 function describe (value, options) {