[1080042][8f245009b0] Big bundle of regexp engine fixes and improvements contributed from Tom Lane of the postgres project.

1 files changed, 1540 insertions, 374 deletions

diff --git a/generic/regc_nfa.c b/generic/regc_nfa.c
index 1fad85f..0e0343e 100644
--- a/generic/regc_nfa.c
+++ b/generic/regc_nfa.c
@@ -34,6 +34,9 @@
 
 #define	NISERR()	VISERR(nfa->v)
 #define	NERR(e)		VERR(nfa->v, (e))
+#define STACK_TOO_DEEP(x) (0)
+#define CANCEL_REQUESTED(x) (0)
+#define REG_CANCEL 777
 
 /*
  - newnfa - set up an NFA
@@ -59,7 +62,6 @@ newnfa(
     nfa->nstates = 0;
     nfa->cm = cm;
     nfa->v = v;
-    nfa->size = 0;
     nfa->bos[0] = nfa->bos[1] = COLORLESS;
     nfa->eos[0] = nfa->eos[1] = COLORLESS;
     nfa->parent = parent;	/* Precedes newfstate so parent is valid. */
@@ -87,61 +89,6 @@ newnfa(
 }
 
 /*
- - TooManyStates - checks if the max states exceeds the compile-time value
- ^ static int TooManyStates(struct nfa *);
- */
-static int
-TooManyStates(
-    struct nfa *nfa)
-{
-    struct nfa *parent = nfa->parent;
-    size_t sz = nfa->size;
-
-    while (parent != NULL) {
-	sz = parent->size;
-	parent = parent->parent;
-    }
-    if (sz > REG_MAX_STATES) {
-	return 1;
-    }
-    return 0;
-}
-
-/*
- - IncrementSize - increases the tracked size of the NFA and its parents.
- ^ static void IncrementSize(struct nfa *);
- */
-static void
-IncrementSize(
-    struct nfa *nfa)
-{
-    struct nfa *parent = nfa->parent;
-
-    nfa->size++;
-    while (parent != NULL) {
-	parent->size++;
-	parent = parent->parent;
-    }
-}
-
-/*
- - DecrementSize - increases the tracked size of the NFA and its parents.
- ^ static void DecrementSize(struct nfa *);
- */
-static void
-DecrementSize(
-    struct nfa *nfa)
-{
-    struct nfa *parent = nfa->parent;
-
-    nfa->size--;
-    while (parent != NULL) {
-	parent->size--;
-	parent = parent->parent;
-    }
-}
-
-/*
  - freenfa - free an entire NFA
  ^ static void freenfa(struct nfa *);
  */
@@ -177,20 +124,20 @@ newstate(
 {
     struct state *s;
 
-    if (TooManyStates(nfa)) {
-	/* XXX: add specific error for this */
-	NERR(REG_ETOOBIG);
-	return NULL;
-    }
     if (nfa->free != NULL) {
 	s = nfa->free;
 	nfa->free = s->next;
     } else {
+	if (nfa->v->spaceused >= REG_MAX_COMPILE_SPACE) {
+	    NERR(REG_ETOOBIG);
+	    return NULL;
+	}
 	s = (struct state *) MALLOC(sizeof(struct state));
 	if (s == NULL) {
 	    NERR(REG_ESPACE);
 	    return NULL;
 	}
+	nfa->v->spaceused += sizeof(struct state);
 	s->oas.next = NULL;
 	s->free = NULL;
 	s->noas = 0;
@@ -214,12 +161,6 @@ newstate(
     }
     s->prev = nfa->slast;
     nfa->slast = s;
-
-    /*
-     * Track the current size and the parent size.
-     */
-
-    IncrementSize(nfa);
     return s;
 }
 
@@ -290,7 +231,6 @@ freestate(
     s->prev = NULL;
     s->next = nfa->free;	/* don't delete it, put it on the free list */
     nfa->free = s;
-    DecrementSize(nfa);
 }
 
 /*
@@ -309,11 +249,13 @@ destroystate(
     for (ab=s->oas.next ; ab!=NULL ; ab=abnext) {
 	abnext = ab->next;
 	FREE(ab);
+	nfa->v->spaceused -= sizeof(struct arcbatch);
     }
     s->ins = NULL;
     s->outs = NULL;
     s->next = NULL;
     FREE(s);
+    nfa->v->spaceused -= sizeof(struct state);
 }
 
 /*
@@ -321,6 +263,10 @@ destroystate(
  ^ static void newarc(struct nfa *, int, pcolor, struct state *,
  ^	struct state *);
  */
+/*
+ * This function checks to make sure that no duplicate arcs are created.
+ * In general we never want duplicates.
+ */
 static void
 newarc(
     struct nfa *nfa,
@@ -333,16 +279,42 @@ newarc(
 
     assert(from != NULL && to != NULL);
 
-    /*
-     * Check for duplicates.
-     */
-
-    for (a=from->outs ; a!=NULL ; a=a->outchain) {
-	if (a->to == to && a->co == co && a->type == t) {
-	    return;
+    /* check for duplicate arc, using whichever chain is shorter */
+    if (from->nouts <= to->nins) {
+	for (a = from->outs; a != NULL; a = a->outchain) {
+	    if (a->to == to && a->co == co && a->type == t) {
+		return;
+	    }
+	}
+    } else {
+	for (a = to->ins; a != NULL; a = a->inchain) {
+	    if (a->from == from && a->co == co && a->type == t) {
+		return;
+	    }
 	}
     }
+  
+    /* no dup, so create the arc */
+    createarc(nfa, t, co, from, to);
+}
+
+/*
+ * createarc - create a new arc within an NFA
+ *
+ * This function must *only* be used after verifying that there is no existing
+ * identical arc (same type/color/from/to).
+ */
+static void
+createarc(
+    struct nfa * nfa,
+    int t,
+    pcolor co,
+    struct state * from,
+    struct state * to)
+{
+    struct arc *a;
 
+    /* the arc is physically allocated within its from-state */
     a = allocarc(nfa, from);
     if (NISERR()) {
 	return;
@@ -355,15 +327,21 @@ newarc(
     a->from = from;
 
     /*
-     * Put the new arc on the beginning, not the end, of the chains. Not only
-     * is this easier, it has the very useful side effect that deleting the
-     * most-recently-added arc is the cheapest case rather than the most
-     * expensive one.
+     * Put the new arc on the beginning, not the end, of the chains; it's
+     * simpler here, and freearc() is the same cost either way.  See also the
+     * logic in moveins() and its cohorts, as well as fixempties().
      */
-
     a->inchain = to->ins;
+    a->inchainRev = NULL;
+    if (to->ins) {
+	to->ins->inchainRev = a;
+    }
     to->ins = a;
     a->outchain = from->outs;
+    a->outchainRev = NULL;
+    if (from->outs) {
+	from->outs->outchainRev = a;
+    }
     from->outs = a;
 
     from->nouts++;
@@ -400,14 +378,19 @@ allocarc(
      */
 
     if (s->free == NULL) {
-	struct arcbatch *newAb = (struct arcbatch *)
-		MALLOC(sizeof(struct arcbatch));
+	struct arcbatch *newAb;
 	int i;
 
+	if (nfa->v->spaceused >= REG_MAX_COMPILE_SPACE) {
+	    NERR(REG_ETOOBIG);
+	    return NULL;
+	}
+	newAb = (struct arcbatch *) MALLOC(sizeof(struct arcbatch));
 	if (newAb == NULL) {
 	    NERR(REG_ESPACE);
 	    return NULL;
 	}
+	nfa->v->spaceused += sizeof(struct arcbatch);
 	newAb->next = s->oas.next;
 	s->oas.next = newAb;
 
@@ -436,7 +419,7 @@ freearc(
 {
     struct state *from = victim->from;
     struct state *to = victim->to;
-    struct arc *a;
+    struct arc *predecessor;
 
     assert(victim->type != 0);
 
@@ -453,16 +436,17 @@ freearc(
      */
 
     assert(from != NULL);
-    assert(from->outs != NULL);
-    a = from->outs;
-    if (a == victim) {		/* simple case: first in chain */
+    predecessor = victim->outchainRev;
+    if (predecessor == NULL) {
+	assert(from->outs == victim);
 	from->outs = victim->outchain;
     } else {
-	for (; a!=NULL && a->outchain!=victim ; a=a->outchain) {
-	    continue;
-	}
-	assert(a != NULL);
-	a->outchain = victim->outchain;
+	assert(predecessor->outchain == victim);
+	predecessor->outchain = victim->outchain;
+    }
+    if (victim->outchain != NULL) {
+	assert(victim->outchain->outchainRev == victim);
+	victim->outchain->outchainRev = predecessor;
     }
     from->nouts--;
 
@@ -471,86 +455,95 @@ freearc(
      */
 
     assert(to != NULL);
-    assert(to->ins != NULL);
-    a = to->ins;
-    if (a == victim) {		/* simple case: first in chain */
+    predecessor = victim->inchainRev;
+    if (predecessor == NULL) {
+	assert(to->ins == victim);
 	to->ins = victim->inchain;
     } else {
-	for (; a->inchain!=victim ; a=a->inchain) {
-	    assert(a->inchain != NULL);
-	    continue;
-	}
-	a->inchain = victim->inchain;
+	assert(predecessor->inchain == victim);
+	predecessor->inchain = victim->inchain;
+    }
+    if (victim->inchain != NULL) {
+	assert(victim->inchain->inchainRev == victim);
+	victim->inchain->inchainRev = predecessor;
     }
     to->nins--;
 
     /*
-     * Clean up and place on free list.
+     * Clean up and place on from-state's free list.
      */
 
     victim->type = 0;
     victim->from = NULL;	/* precautions... */
     victim->to = NULL;
     victim->inchain = NULL;
+    victim->inchainRev = NULL;
     victim->outchain = NULL;
+    victim->outchainRev = NULL;
     victim->freechain = from->free;
     from->free = victim;
 }
-
+
 /*
- - hasnonemptyout - Does state have a non-EMPTY out arc?
- ^ static int hasnonemptyout(struct state *);
+ * changearctarget - flip an arc to have a different to state
+ *
+ * Caller must have verified that there is no pre-existing duplicate arc.
+ *
+ * Note that because we store arcs in their from state, we can't easily have
+ * a similar changearcsource function.
  */
-static int
-hasnonemptyout(
-    struct state *s)
+static void
+changearctarget(struct arc * a, struct state * newto)
 {
-    struct arc *a;
+    struct state *oldto = a->to;
+    struct arc *predecessor;
 
-    for (a = s->outs; a != NULL; a = a->outchain) {
-	if (a->type != EMPTY) {
-	    return 1;
-	}
+    assert(oldto != newto);
+
+    /* take it off old target's in-chain */
+    assert(oldto != NULL);
+    predecessor = a->inchainRev;
+    if (predecessor == NULL) {
+	assert(oldto->ins == a);
+	oldto->ins = a->inchain;
+    } else {
+	assert(predecessor->inchain == a);
+	predecessor->inchain = a->inchain;
     }
-    return 0;
-}
-
-/*
- - nonemptyouts - count non-EMPTY out arcs of a state
- ^ static int nonemptyouts(struct state *);
- */
-static int
-nonemptyouts(
-    struct state *s)
-{
-    int n = 0;
-    struct arc *a;
+    if (a->inchain != NULL) {
+	assert(a->inchain->inchainRev == a);
+	a->inchain->inchainRev = predecessor;
+    }
+    oldto->nins--;
 
-    for (a = s->outs; a != NULL; a = a->outchain) {
-	if (a->type != EMPTY) {
-	    n++;
-	}
+    a->to = newto;
+
+    /* prepend it to new target's in-chain */
+    a->inchain = newto->ins;
+    a->inchainRev = NULL;
+    if (newto->ins) {
+	newto->ins->inchainRev = a;
     }
-    return n;
+    newto->ins = a;
+    newto->nins++;
 }
 
 /*
- - nonemptyins - count non-EMPTY in arcs of a state
- ^ static int nonemptyins(struct state *);
+ - hasnonemptyout - Does state have a non-EMPTY out arc?
+ ^ static int hasnonemptyout(struct state *);
  */
 static int
-nonemptyins(
+hasnonemptyout(
     struct state *s)
 {
-    int n = 0;
     struct arc *a;
 
-    for (a = s->ins; a != NULL; a = a->inchain) {
+    for (a = s->outs; a != NULL; a = a->outchain) {
 	if (a->type != EMPTY) {
-	    n++;
+	    return 1;
 	}
     }
-    return n;
+    return 0;
 }
 
 /*
@@ -588,13 +581,181 @@ cparc(
 {
     newarc(nfa, oa->type, oa->co, from, to);
 }
+
+/*
+ * sortins - sort the in arcs of a state by from/color/type
+ */
+static void
+sortins(
+    struct nfa * nfa,
+    struct state * s)
+{
+    struct arc **sortarray;
+    struct arc *a;
+    int n = s->nins;
+    int i;
+
+    if (n <= 1) {
+	return;		/* nothing to do */
+    }
+    /* make an array of arc pointers ... */
+    sortarray = (struct arc **) MALLOC(n * sizeof(struct arc *));
+    if (sortarray == NULL) {
+	NERR(REG_ESPACE);
+	return;
+    }
+    i = 0;
+    for (a = s->ins; a != NULL; a = a->inchain) {
+	sortarray[i++] = a;
+    }
+    assert(i == n);
+    /* ... sort the array */
+    qsort(sortarray, n, sizeof(struct arc *), sortins_cmp);
+    /* ... and rebuild arc list in order */
+    /* it seems worth special-casing first and last items to simplify loop */
+    a = sortarray[0];
+    s->ins = a;
+    a->inchain = sortarray[1];
+    a->inchainRev = NULL;
+    for (i = 1; i < n - 1; i++) {
+	a = sortarray[i];
+	a->inchain = sortarray[i + 1];
+	a->inchainRev = sortarray[i - 1];
+    }
+    a = sortarray[i];
+    a->inchain = NULL;
+    a->inchainRev = sortarray[i - 1];
+    FREE(sortarray);
+}
+
+static int
+sortins_cmp(
+    const void *a,
+    const void *b)
+{
+    const struct arc *aa = *((const struct arc * const *) a);
+    const struct arc *bb = *((const struct arc * const *) b);
+
+    /* we check the fields in the order they are most likely to be different */
+    if (aa->from->no < bb->from->no) {
+	return -1;
+    }
+    if (aa->from->no > bb->from->no) {
+ 	return 1;
+    }
+    if (aa->co < bb->co) {
+ 	return -1;
+    }
+    if (aa->co > bb->co) {
+ 	return 1;
+    }
+    if (aa->type < bb->type) {
+ 	return -1;
+    }
+    if (aa->type > bb->type) {
+ 	return 1;
+    }
+    return 0;
+}
+ 
+/*
+ * sortouts - sort the out arcs of a state by to/color/type
+ */
+static void
+sortouts(
+    struct nfa * nfa,
+    struct state * s)
+{
+    struct arc **sortarray;
+    struct arc *a;
+    int	n = s->nouts;
+    int	i;
+
+    if (n <= 1) {
+	return;					/* nothing to do */
+    }
+    /* make an array of arc pointers ... */
+    sortarray = (struct arc **) MALLOC(n * sizeof(struct arc *));
+    if (sortarray == NULL) {
+	NERR(REG_ESPACE);
+	return;
+    }
+    i = 0;
+    for (a = s->outs; a != NULL; a = a->outchain) {
+	sortarray[i++] = a;
+    }
+    assert(i == n);
+    /* ... sort the array */
+    qsort(sortarray, n, sizeof(struct arc *), sortouts_cmp);
+    /* ... and rebuild arc list in order */
+    /* it seems worth special-casing first and last items to simplify loop */
+    a = sortarray[0];
+    s->outs = a;
+    a->outchain = sortarray[1];
+    a->outchainRev = NULL;
+    for (i = 1; i < n - 1; i++) {
+	a = sortarray[i];
+	a->outchain = sortarray[i + 1];
+	a->outchainRev = sortarray[i - 1];
+    }
+    a = sortarray[i];
+    a->outchain = NULL;
+    a->outchainRev = sortarray[i - 1];
+    FREE(sortarray);
+}
+
+static int
+sortouts_cmp(
+    const void *a,
+    const void *b)
+{
+    const struct arc *aa = *((const struct arc * const *) a);
+    const struct arc *bb = *((const struct arc * const *) b);
+
+    /* we check the fields in the order they are most likely to be different */
+    if (aa->to->no < bb->to->no) {
+	return -1;
+    }
+    if (aa->to->no > bb->to->no) {
+	return 1;
+    }
+    if (aa->co < bb->co) {
+	return -1;
+    }
+    if (aa->co > bb->co) {
+	return 1;
+    }
+    if (aa->type < bb->type) {
+	return -1;
+    }
+    if (aa->type > bb->type) {
+	return 1;
+    }
+    return 0;
+}
+
+/*
+ * Common decision logic about whether to use arc-by-arc operations or
+ * sort/merge.  If there's just a few source arcs we cannot recoup the
+ * cost of sorting the destination arc list, no matter how large it is.
+ * Otherwise, limit the number of arc-by-arc comparisons to about 1000
+ * (a somewhat arbitrary choice, but the breakeven point would probably
+ * be machine dependent anyway).
+ */
+#define BULK_ARC_OP_USE_SORT(nsrcarcs, ndestarcs) \
+	((nsrcarcs) < 4 ? 0 : ((nsrcarcs) > 32 || (ndestarcs) > 32))
 
 /*
  - moveins - move all in arcs of a state to another state
  * You might think this could be done better by just updating the
- * existing arcs, and you would be right if it weren't for the desire
+ * existing arcs, and you would be right if it weren't for the need
  * for duplicate suppression, which makes it easier to just make new
  * ones to exploit the suppression built into newarc.
+ *
+ * However, if we have a whole lot of arcs to deal with, retail duplicate
+ * checks become too slow.  In that case we proceed by sorting and merging
+ * the arc lists, and then we can indeed just update the arcs in-place.
+ *
  ^ static void moveins(struct nfa *, struct state *, struct state *);
  */
 static void
@@ -603,38 +764,253 @@ moveins(
     struct state *oldState,
     struct state *newState)
 {
-    struct arc *a;
-
     assert(oldState != newState);
 
-    while ((a = oldState->ins) != NULL) {
-	cparc(nfa, a, a->from, newState);
-	freearc(nfa, a);
+    if (!BULK_ARC_OP_USE_SORT(oldState->nins, newState->nins)) {
+	/* With not too many arcs, just do them one at a time */
+	struct arc *a;
+
+	while ((a = oldState->ins) != NULL) {
+	    cparc(nfa, a, a->from, newState);
+	    freearc(nfa, a);
+	}
+    } else {
+	/*
+	 * With many arcs, use a sort-merge approach.  Note changearctarget()
+	 * will put the arc onto the front of newState's chain, so it does not
+	 * break our walk through the sorted part of the chain.
+	 */
+	struct arc *oa;
+	struct arc *na;
+
+	/*
+	 * Because we bypass newarc() in this code path, we'd better include a
+	 * cancel check.
+	 */
+	if (CANCEL_REQUESTED(nfa->v->re)) {
+	    NERR(REG_CANCEL);
+	    return;
+	}
+
+	sortins(nfa, oldState);
+	sortins(nfa, newState);
+	if (NISERR()) {
+	    return;		/* might have failed to sort */
+	}
+	oa = oldState->ins;
+	na = newState->ins;
+	while (oa != NULL && na != NULL) {
+	    struct arc *a = oa;
+
+	    switch (sortins_cmp(&oa, &na)) {
+		case -1:
+		    /* newState does not have anything matching oa */
+		    oa = oa->inchain;
+
+		    /*
+		     * Rather than doing createarc+freearc, we can just unlink
+		     * and relink the existing arc struct.
+		     */
+		    changearctarget(a, newState);
+		    break;
+		case 0:
+		    /* match, advance in both lists */
+		    oa = oa->inchain;
+		    na = na->inchain;
+		    /* ... and drop duplicate arc from oldState */
+		    freearc(nfa, a);
+		    break;
+		case +1:
+		    /* advance only na; oa might have a match later */
+		    na = na->inchain;
+		    break;
+		default:
+		    assert(NOTREACHED);
+	    }
+	}
+	while (oa != NULL) {
+	    /* newState does not have anything matching oa */
+	    struct arc *a = oa;
+
+	    oa = oa->inchain;
+	    changearctarget(a, newState);
+	}
     }
+
     assert(oldState->nins == 0);
     assert(oldState->ins == NULL);
 }
 
 /*
  - copyins - copy in arcs of a state to another state
- * Either all arcs, or only non-empty ones as determined by all value.
  ^ static VOID copyins(struct nfa *, struct state *, struct state *, int);
  */
 static void
 copyins(
     struct nfa *nfa,
     struct state *oldState,
-    struct state *newState,
-    int all)
+    struct state *newState)
 {
-    struct arc *a;
-
     assert(oldState != newState);
 
-    for (a=oldState->ins ; a!=NULL ; a=a->inchain) {
-	if (all || a->type != EMPTY) {
+    if (!BULK_ARC_OP_USE_SORT(oldState->nins, newState->nins)) {
+	/* With not too many arcs, just do them one at a time */
+	struct arc *a;
+
+	for (a = oldState->ins; a != NULL; a = a->inchain) {
 	    cparc(nfa, a, a->from, newState);
 	}
+    } else {
+	/*
+	 * With many arcs, use a sort-merge approach.  Note that createarc()
+	 * will put new arcs onto the front of newState's chain, so it does
+	 * not break our walk through the sorted part of the chain.
+	 */
+	struct arc *oa;
+	struct arc *na;
+
+	/*
+	 * Because we bypass newarc() in this code path, we'd better include a
+	 * cancel check.
+	 */
+	if (CANCEL_REQUESTED(nfa->v->re)) {
+	    NERR(REG_CANCEL);
+	    return;
+	}
+
+	sortins(nfa, oldState);
+	sortins(nfa, newState);
+	if (NISERR()) {
+	    return;		/* might have failed to sort */
+	}
+	oa = oldState->ins;
+	na = newState->ins;
+	while (oa != NULL && na != NULL) {
+	    struct arc *a = oa;
+
+	    switch (sortins_cmp(&oa, &na)) {
+		case -1:
+		    /* newState does not have anything matching oa */
+		    oa = oa->inchain;
+		    createarc(nfa, a->type, a->co, a->from, newState);
+		    break;
+		case 0:
+		    /* match, advance in both lists */
+		    oa = oa->inchain;
+		    na = na->inchain;
+		    break;
+		case +1:
+		    /* advance only na; oa might have a match later */
+		    na = na->inchain;
+		    break;
+		default:
+		    assert(NOTREACHED);
+	    }
+	}
+	while (oa != NULL) {
+	    /* newState does not have anything matching oa */
+	    struct arc *a = oa;
+
+	    oa = oa->inchain;
+	    createarc(nfa, a->type, a->co, a->from, newState);
+	}
+    }
+}
+
+/*
+ * mergeins - merge a list of inarcs into a state
+ *
+ * This is much like copyins, but the source arcs are listed in an array,
+ * and are not guaranteed unique.  It's okay to clobber the array contents.
+ */
+static void
+mergeins(
+    struct nfa * nfa,
+    struct state * s,
+    struct arc ** arcarray,
+    int arccount)
+{
+    struct arc *na;
+    int	i;
+    int	j;
+
+    if (arccount <= 0) {
+	return;
+    }
+
+    /*
+     * Because we bypass newarc() in this code path, we'd better include a
+     * cancel check.
+     */
+    if (CANCEL_REQUESTED(nfa->v->re)) {
+	NERR(REG_CANCEL);
+	return;
+    }
+
+    /* Sort existing inarcs as well as proposed new ones */
+    sortins(nfa, s);
+    if (NISERR()) {
+	return;			/* might have failed to sort */
+    }
+
+    qsort(arcarray, arccount, sizeof(struct arc *), sortins_cmp);
+
+    /*
+     * arcarray very likely includes dups, so we must eliminate them.  (This
+     * could be folded into the next loop, but it's not worth the trouble.)
+     */
+    j = 0;
+    for (i = 1; i < arccount; i++) {
+	switch (sortins_cmp(&arcarray[j], &arcarray[i])) {
+	    case -1:
+		/* non-dup */
+		arcarray[++j] = arcarray[i];
+		break;
+	    case 0:
+		/* dup */
+		break;
+	    default:
+		/* trouble */
+		assert(NOTREACHED);
+	}
+    }
+    arccount = j + 1;
+
+    /*
+     * Now merge into s' inchain.  Note that createarc() will put new arcs
+     * onto the front of s's chain, so it does not break our walk through the
+     * sorted part of the chain.
+     */
+    i = 0;
+    na = s->ins;
+    while (i < arccount && na != NULL) {
+	struct arc *a = arcarray[i];
+
+	switch (sortins_cmp(&a, &na)) {
+	    case -1:
+		/* s does not have anything matching a */
+		createarc(nfa, a->type, a->co, a->from, s);
+		i++;
+		break;
+	    case 0:
+		/* match, advance in both lists */
+		i++;
+		na = na->inchain;
+		break;
+	    case +1:
+		/* advance only na; array might have a match later */
+		na = na->inchain;
+		break;
+	    default:
+		assert(NOTREACHED);
+	}
+    }
+    while (i < arccount) {
+	/* s does not have anything matching a */
+	struct arc *a = arcarray[i];
+
+	createarc(nfa, a->type, a->co, a->from, s);
+	i++;
     }
 }
 
@@ -648,36 +1024,153 @@ moveouts(
     struct state *oldState,
     struct state *newState)
 {
-    struct arc *a;
-
     assert(oldState != newState);
 
-    while ((a = oldState->outs) != NULL) {
-	cparc(nfa, a, newState, a->to);
-	freearc(nfa, a);
+    if (!BULK_ARC_OP_USE_SORT(oldState->nouts, newState->nouts)) {
+	/* With not too many arcs, just do them one at a time */
+	struct arc *a;
+
+	while ((a = oldState->outs) != NULL) {
+	    cparc(nfa, a, newState, a->to);
+	    freearc(nfa, a);
+	}
+    } else {
+	/*
+	 * With many arcs, use a sort-merge approach.  Note that createarc()
+	 * will put new arcs onto the front of newState's chain, so it does
+	 * not break our walk through the sorted part of the chain.
+	 */
+	struct arc *oa;
+	struct arc *na;
+
+	/*
+	 * Because we bypass newarc() in this code path, we'd better include a
+	 * cancel check.
+	 */
+	if (CANCEL_REQUESTED(nfa->v->re)) {
+	    NERR(REG_CANCEL);
+	    return;
+	}
+
+	sortouts(nfa, oldState);
+	sortouts(nfa, newState);
+	if (NISERR()) {
+	    return;	/* might have failed to sort */
+	}
+	oa = oldState->outs;
+	na = newState->outs;
+	while (oa != NULL && na != NULL) {
+	    struct arc *a = oa;
+
+	    switch (sortouts_cmp(&oa, &na)) {
+		case -1:
+		    /* newState does not have anything matching oa */
+		    oa = oa->outchain;
+		    createarc(nfa, a->type, a->co, newState, a->to);
+		    freearc(nfa, a);
+		    break;
+		case 0:
+		    /* match, advance in both lists */
+		    oa = oa->outchain;
+		    na = na->outchain;
+		    /* ... and drop duplicate arc from oldState */
+		    freearc(nfa, a);
+		    break;
+		case +1:
+		    /* advance only na; oa might have a match later */
+		    na = na->outchain;
+		    break;
+		default:
+		    assert(NOTREACHED);
+	    }
+	}
+	while (oa != NULL) {
+	    /* newState does not have anything matching oa */
+	    struct arc *a = oa;
+
+	    oa = oa->outchain;
+	    createarc(nfa, a->type, a->co, newState, a->to);
+	    freearc(nfa, a);
+	}
     }
+
+    assert(oldState->nouts == 0);
+    assert(oldState->outs == NULL);
 }
 
 /*
  - copyouts - copy out arcs of a state to another state
- * Either all arcs, or only non-empty ones as determined by all value.
  ^ static VOID copyouts(struct nfa *, struct state *, struct state *, int);
  */
 static void
 copyouts(
     struct nfa *nfa,
     struct state *oldState,
-    struct state *newState,
-    int all)
+    struct state *newState)
 {
-    struct arc *a;
-
     assert(oldState != newState);
 
-    for (a=oldState->outs ; a!=NULL ; a=a->outchain) {
-	if (all || a->type != EMPTY) {
+    if (!BULK_ARC_OP_USE_SORT(oldState->nouts, newState->nouts)) {
+	/* With not too many arcs, just do them one at a time */
+	struct arc *a;
+
+	for (a = oldState->outs; a != NULL; a = a->outchain) {
 	    cparc(nfa, a, newState, a->to);
 	}
+    } else {
+ 	/*
+	 * With many arcs, use a sort-merge approach.  Note that createarc()
+	 * will put new arcs onto the front of newState's chain, so it does
+	 * not break our walk through the sorted part of the chain.
+	 */
+	struct arc *oa;
+	struct arc *na;
+
+	/*
+	 * Because we bypass newarc() in this code path, we'd better include a
+	 * cancel check.
+	 */
+	if (CANCEL_REQUESTED(nfa->v->re)) {
+	    NERR(REG_CANCEL);
+	    return;
+	}
+
+	sortouts(nfa, oldState);
+	sortouts(nfa, newState);
+	if (NISERR()) {
+	    return;		/* might have failed to sort */
+	}
+	oa = oldState->outs;
+	na = newState->outs;
+	while (oa != NULL && na != NULL) {
+	    struct arc *a = oa;
+
+	    switch (sortouts_cmp(&oa, &na)) {
+		case -1:
+		    /* newState does not have anything matching oa */
+		    oa = oa->outchain;
+		    createarc(nfa, a->type, a->co, newState, a->to);
+		    break;
+		case 0:
+		    /* match, advance in both lists */
+		    oa = oa->outchain;
+		    na = na->outchain;
+		    break;
+		case +1:
+		    /* advance only na; oa might have a match later */
+		    na = na->outchain;
+		    break;
+		default:
+		    assert(NOTREACHED);
+	    }
+	}
+	while (oa != NULL) {
+	    /* newState does not have anything matching oa */
+	    struct arc *a = oa;
+
+	    oa = oa->outchain;
+	    createarc(nfa, a->type, a->co, newState, a->to);
+	}
     }
 }
 
@@ -896,6 +1389,20 @@ specialcolors(
  - optimize - optimize an NFA
  ^ static long optimize(struct nfa *, FILE *);
  */
+
+ /*
+  * The main goal of this function is not so much "optimization" (though it
+  * does try to get rid of useless NFA states) as reducing the NFA to a form
+  * the regex executor can handle.  The executor, and indeed the cNFA format
+  * that is its input, can only handle PLAIN and LACON arcs.  The output of
+  * the regex parser also includes EMPTY (do-nothing) arcs, as well as
+  * ^, $, AHEAD, and BEHIND constraint arcs, which we must get rid of here.
+  * We first get rid of EMPTY arcs and then deal with the constraint arcs.
+  * The hardest part of either job is to get rid of circular loops of the
+  * target arc type.  We would have to do that in any case, though, as such a
+  * loop would otherwise allow the executor to cycle through the loop endlessly
+  * without making any progress in the input string.
+  */
 static long			/* re_info bits */
 optimize(
     struct nfa *nfa,
@@ -917,17 +1424,23 @@ optimize(
     if (verbose) {
 	fprintf(f, "\nconstraints:\n");
     }
+    fixconstraintloops(nfa, f);	/* get rid of constraint loops */
     pullback(nfa, f);		/* pull back constraints backward */
     pushfwd(nfa, f);		/* push fwd constraints forward */
     if (verbose) {
 	fprintf(f, "\nfinal cleanup:\n");
     }
     cleanup(nfa);		/* final tidying */
+#ifdef REG_DEBUG
+    if (verbose) {
+	dumpnfa(nfa, f);
+    }
+#endif
     return analyze(nfa);	/* and analysis */
 }
 
 /*
- - pullback - pull back constraints backward to (with luck) eliminate them
+ - pullback - pull back constraints backward to eliminate them
  ^ static void pullback(struct nfa *, FILE *);
  */
 static void
@@ -939,6 +1452,7 @@ pullback(
     struct state *nexts;
     struct arc *a;
     struct arc *nexta;
+    struct state *intermediates;
     int progress;
 
     /*
@@ -949,15 +1463,27 @@ pullback(
 	progress = 0;
 	for (s=nfa->states ; s!=NULL && !NISERR() ; s=nexts) {
 	    nexts = s->next;
+	    intermediates = NULL;
 	    for (a=s->outs ; a!=NULL && !NISERR() ; a=nexta) {
 		nexta = a->outchain;
 		if (a->type == '^' || a->type == BEHIND) {
-		    if (pull(nfa, a)) {
+		    if (pull(nfa, a, &intermediates)) {
 			progress = 1;
 		    }
 		}
 		assert(nexta == NULL || s->no != FREESTATE);
 	    }
+	    /* clear tmp fields of intermediate states created here */
+	    while (intermediates != NULL) {
+		struct state *ns = intermediates->tmp;
+
+		intermediates->tmp = NULL;
+		intermediates = ns;
+	    }
+	    /* if s is now useless, get rid of it */
+	    if ((s->nins == 0 || s->nouts == 0) && !s->flag) {
+		dropstate(nfa, s);
+	    }
 	}
 	if (progress && f != NULL) {
 	    dumpnfa(nfa, f);
@@ -967,6 +1493,12 @@ pullback(
 	return;
     }
 
+    /*
+     * Any ^ constraints we were able to pull to the start state can now be
+     * replaced by PLAIN arcs referencing the BOS or BOL colors.  There should
+     * be no other ^ or BEHIND arcs left in the NFA, though we do not check
+     * that here (compact() will fail if so).
+     */
     for (a=nfa->pre->outs ; a!=NULL ; a=nexta) {
 	nexta = a->outchain;
 	if (a->type == '^') {
@@ -979,15 +1511,28 @@ pullback(
 
 /*
  - pull - pull a back constraint backward past its source state
- * A significant property of this function is that it deletes at most
- * one state -- the constraint's from state -- and only if the constraint
- * was that state's last outarc.
+ *
+ * Returns 1 if successful (which it always is unless the source is the
+ * start state or we have an internal error), 0 if nothing happened.
+ *
+ * A significant property of this function is that it deletes no pre-existing
+ * states, and no outarcs of the constraint's from state other than the given
+ * constraint arc.  This makes the loops in pullback() safe, at the cost that
+ * we may leave useless states behind.  Therefore, we leave it to pullback()
+ * to delete such states.
+ *
+ * If the from state has multiple back-constraint outarcs, and/or multiple
+ * compatible constraint inarcs, we only need to create one new intermediate
+ * state per combination of predecessor and successor states.  *intermediates
+ * points to a list of such intermediate states for this from state (chained
+ * through their tmp fields).
  ^ static int pull(struct nfa *, struct arc *);
  */
-static int			/* 0 couldn't, 1 could */
+static int
 pull(
     struct nfa *nfa,
-    struct arc *con)
+    struct arc *con,
+    struct state **intermediates)
 {
     struct state *from = con->from;
     struct state *to = con->to;
@@ -995,10 +1540,7 @@ pull(
     struct arc *nexta;
     struct state *s;
 
-    if (from == to) {		/* circular constraint is pointless */
-	freearc(nfa, con);
-	return 1;
-    }
+    assert(from != to);		/* should have gotten rid of this earlier */
     if (from->flag) {		/* can't pull back beyond start */
 	return 0;
     }
@@ -1008,26 +1550,9 @@ pull(
     }
 
     /*
-     * DGP 2007-11-15: Cloning a state with a circular constraint on its list
-     * of outs can lead to trouble [Bug 1810038], so get rid of them first.
-     */
-
-    for (a = from->outs; a != NULL; a = nexta) {
-	nexta = a->outchain;
-	switch (a->type) {
-	case '^':
-	case '$':
-	case BEHIND:
-	case AHEAD:
-	    if (from == a->to) {
-		freearc(nfa, a);
-	    }
-	    break;
-	}
-    }
-
-    /*
-     * First, clone from state if necessary to avoid other outarcs.
+     * First, clone from state if necessary to avoid other outarcs.  This may
+     * seem wasteful, but it simplifies the logic, and we'll get rid of the
+     * clone state again at the bottom.
      */
 
     if (from->nouts > 1) {
@@ -1035,10 +1560,12 @@ pull(
 	if (NISERR()) {
 	    return 0;
 	}
-	assert(to != from);		/* con is not an inarc */
-	copyins(nfa, from, s, 1);	/* duplicate inarcs */
+	copyins(nfa, from, s);	/* duplicate inarcs */
 	cparc(nfa, con, s, to);		/* move constraint arc */
 	freearc(nfa, con);
+	if (NISERR()) {
+	    return 0;
+	}
 	from = s;
 	con = from->outs;
     }
@@ -1048,7 +1575,7 @@ pull(
      * Propagate the constraint into the from state's inarcs.
      */
 
-    for (a=from->ins ; a!=NULL ; a=nexta) {
+    for (a=from->ins ; a!=NULL && !NISERR(); a=nexta) {
 	nexta = a->inchain;
 	switch (combine(con, a)) {
 	case INCOMPATIBLE:	/* destroy the arc */
@@ -1057,17 +1584,25 @@ pull(
 	case SATISFIED:		/* no action needed */
 	    break;
 	case COMPATIBLE:	/* swap the two arcs, more or less */
-	    s = newstate(nfa);
-	    if (NISERR()) {
-		return 0;
+	    /* need an intermediate state, but might have one already */
+	    for (s = *intermediates; s != NULL; s = s->tmp) {
+		assert(s->nins > 0 && s->nouts > 0);
+		if (s->ins->from == a->from && s->outs->to == to) {
+		    break;
+		}
 	    }
-	    cparc(nfa, a, s, to);	/* anticipate move */
-	    cparc(nfa, con, a->from, s);
-	    if (NISERR()) {
-		return 0;
+	    if (s == NULL) {
+		s = newstate(nfa);
+		if (NISERR()) {
+		    return 0;
+		}
+		s->tmp = *intermediates;
+		*intermediates = s;
 	    }
-	    freearc(nfa, a);
-	    break;
+  	    cparc(nfa, con, a->from, s);
+	    cparc(nfa, a, s, to);
+ 	    freearc(nfa, a);
+  	    break;
 	default:
 	    assert(NOTREACHED);
 	    break;
@@ -1079,12 +1614,13 @@ pull(
      */
 
     moveins(nfa, from, to);
-    dropstate(nfa, from);	/* will free the constraint */
+    freearc(nfa, con);
+    /* from state is now useless, but we leave it to pullback() to clean up */
     return 1;
 }
 
 /*
- - pushfwd - push forward constraints forward to (with luck) eliminate them
+ - pushfwd - push forward constraints forward to eliminate them
  ^ static void pushfwd(struct nfa *, FILE *);
  */
 static void
@@ -1096,6 +1632,7 @@ pushfwd(
     struct state *nexts;
     struct arc *a;
     struct arc *nexta;
+    struct state *intermediates;
     int progress;
 
     /*
@@ -1106,14 +1643,25 @@ pushfwd(
 	progress = 0;
 	for (s=nfa->states ; s!=NULL && !NISERR() ; s=nexts) {
 	    nexts = s->next;
+	    intermediates = NULL;
 	    for (a = s->ins; a != NULL && !NISERR(); a = nexta) {
 		nexta = a->inchain;
 		if (a->type == '$' || a->type == AHEAD) {
-		    if (push(nfa, a)) {
+		    if (push(nfa, a, &intermediates)) {
 			progress = 1;
 		    }
 		}
-		assert(nexta == NULL || s->no != FREESTATE);
+	    }
+	    /* clear tmp fields of intermediate states created here */
+	    while (intermediates != NULL) {
+		struct state *ns = intermediates->tmp;
+
+		intermediates->tmp = NULL;
+		intermediates = ns;
+	    }
+	    /* if s is now useless, get rid of it */
+	    if ((s->nins == 0 || s->nouts == 0) && !s->flag) {
+		dropstate(nfa, s);
 	    }
 	}
 	if (progress && f != NULL) {
@@ -1124,6 +1672,12 @@ pushfwd(
 	return;
     }
 
+    /*
+     * Any $ constraints we were able to push to the post state can now be
+     * replaced by PLAIN arcs referencing the EOS or EOL colors.  There should
+     * be no other $ or AHEAD arcs left in the NFA, though we do not check
+     * that here (compact() will fail if so).
+     */
     for (a = nfa->post->ins; a != NULL; a = nexta) {
 	nexta = a->inchain;
 	if (a->type == '$') {
@@ -1136,15 +1690,28 @@ pushfwd(
 
 /*
  - push - push a forward constraint forward past its destination state
- * A significant property of this function is that it deletes at most
- * one state -- the constraint's to state -- and only if the constraint
- * was that state's last inarc.
+ *
+ * Returns 1 if successful (which it always is unless the destination is the
+ * post state or we have an internal error), 0 if nothing happened.
+ *
+ * A significant property of this function is that it deletes no pre-existing
+ * states, and no inarcs of the constraint's to state other than the given
+ * constraint arc.  This makes the loops in pushfwd() safe, at the cost that
+ * we may leave useless states behind.  Therefore, we leave it to pushfwd()
+ * to delete such states.
+ *
+ * If the to state has multiple forward-constraint inarcs, and/or multiple
+ * compatible constraint outarcs, we only need to create one new intermediate
+ * state per combination of predecessor and successor states.  *intermediates
+ * points to a list of such intermediate states for this to state (chained
+ * through their tmp fields).
  ^ static int push(struct nfa *, struct arc *);
  */
-static int			/* 0 couldn't, 1 could */
+static int
 push(
     struct nfa *nfa,
-    struct arc *con)
+    struct arc *con,
+    struct state **intermediates)
 {
     struct state *from = con->from;
     struct state *to = con->to;
@@ -1152,10 +1719,7 @@ push(
     struct arc *nexta;
     struct state *s;
 
-    if (to == from) {		/* circular constraint is pointless */
-	freearc(nfa, con);
-	return 1;
-    }
+    assert(to != from);		/* should have gotten rid of this earlier */
     if (to->flag) {		/* can't push forward beyond end */
 	return 0;
     }
@@ -1165,29 +1729,9 @@ push(
     }
 
     /*
-     * DGP 2007-11-15: Here we duplicate the same protections as appear
-     * in pull() above to avoid troubles with cloning a state with a
-     * circular constraint on its list of ins.  It is not clear whether
-     * this is necessary, or is protecting against a "can't happen".
-     * Any test case that actually leads to a freearc() call here would
-     * be a welcome addition to the test suite.
-     */
-
-    for (a = to->ins; a != NULL; a = nexta) {
-	nexta = a->inchain;
-	switch (a->type) {
-	case '^':
-	case '$':
-	case BEHIND:
-	case AHEAD:
-	    if (a->from == to) {
-		freearc(nfa, a);
-	    }
-	    break;
-	}
-    }
-    /*
-     * First, clone to state if necessary to avoid other inarcs.
+     * First, clone to state if necessary to avoid other inarcs.  This may
+     * seem wasteful, but it simplifies the logic, and we'll get rid of the
+     * clone state again at the bottom.
      */
 
     if (to->nins > 1) {
@@ -1195,9 +1739,12 @@ push(
 	if (NISERR()) {
 	    return 0;
 	}
-	copyouts(nfa, to, s, 1);	/* duplicate outarcs */
-	cparc(nfa, con, from, s);	/* move constraint */
+	copyouts(nfa, to, s);		/* duplicate outarcs */
+	cparc(nfa, con, from, s);	/* move constraint arc */
 	freearc(nfa, con);
+	if (NISERR()) {
+	    return 0;
+	}
 	to = s;
 	con = to->ins;
     }
@@ -1207,7 +1754,7 @@ push(
      * Propagate the constraint into the to state's outarcs.
      */
 
-    for (a = to->outs; a != NULL; a = nexta) {
+    for (a = to->outs; a != NULL && !NISERR(); a = nexta) {
 	nexta = a->outchain;
 	switch (combine(con, a)) {
 	case INCOMPATIBLE:	/* destroy the arc */
@@ -1216,17 +1763,25 @@ push(
 	case SATISFIED:		/* no action needed */
 	    break;
 	case COMPATIBLE:	/* swap the two arcs, more or less */
-	    s = newstate(nfa);
-	    if (NISERR()) {
-		return 0;
+	    /* need an intermediate state, but might have one already */
+	    for (s = *intermediates; s != NULL; s = s->tmp) {
+		assert(s->nins > 0 && s->nouts > 0);
+		if (s->ins->from == from && s->outs->to == a->to) {
+		    break;
+		}
 	    }
-	    cparc(nfa, con, s, a->to);	/* anticipate move */
-	    cparc(nfa, a, from, s);
-	    if (NISERR()) {
-		return 0;
+	    if (s == NULL) {
+		s = newstate(nfa);
+		if (NISERR()) {
+		    return 0;
+		}
+		s->tmp = *intermediates;
+		*intermediates = s;
 	    }
-	    freearc(nfa, a);
-	    break;
+	    cparc(nfa, con, s, a->to);
+  	    cparc(nfa, a, from, s);
+  	    freearc(nfa, a);
+  	    break;
 	default:
 	    assert(NOTREACHED);
 	    break;
@@ -1238,7 +1793,8 @@ push(
      */
 
     moveouts(nfa, to, from);
-    dropstate(nfa, to);		/* will free the constraint */
+    freearc(nfa, con);
+    /* to state is now useless, but we leave it to pushfwd() to clean up */
     return 1;
 }
 
@@ -1316,6 +1872,12 @@ fixempties(
     struct state *nexts;
     struct arc *a;
     struct arc *nexta;
+    int totalinarcs;
+    struct arc **inarcsorig;
+    struct arc **arcarray;
+    int arccount;
+    int prevnins;
+    int nskip;
 
     /*
      * First, get rid of any states whose sole out-arc is an EMPTY,
@@ -1361,42 +1923,129 @@ fixempties(
 	dropstate(nfa, s);
     }
 
+    if (NISERR()) {
+	return;
+    }
+
     /*
-     * For each remaining NFA state, find all other states that are
-     * reachable from it by a chain of one or more EMPTY arcs.  Then
-     * generate new arcs that eliminate the need for each such chain.
+     * For each remaining NFA state, find all other states from which it is
+     * reachable by a chain of one or more EMPTY arcs.  Then generate new arcs
+     * that eliminate the need for each such chain.
+     *
+     * We could replace a chain of EMPTY arcs that leads from a "from" state
+     * to a "to" state either by pushing non-EMPTY arcs forward (linking
+     * directly from "from"'s predecessors to "to") or by pulling them back
+     * (linking directly from "from" to "to"'s successors).  We choose to
+     * always do the former; this choice is somewhat arbitrary, but the
+     * approach below requires that we uniformly do one or the other.
+     *
+     * Suppose we have a chain of N successive EMPTY arcs (where N can easily
+     * approach the size of the NFA).  All of the intermediate states must
+     * have additional inarcs and outarcs, else they'd have been removed by
+     * the steps above.  Assuming their inarcs are mostly not empties, we will
+     * add O(N^2) arcs to the NFA, since a non-EMPTY inarc leading to any one
+     * state in the chain must be duplicated to lead to all its successor
+     * states as well.  So there is no hope of doing less than O(N^2) work;
+     * however, we should endeavor to keep the big-O cost from being even
+     * worse than that, which it can easily become without care.  In
+     * particular, suppose we were to copy all S1's inarcs forward to S2, and
+     * then also to S3, and then later we consider pushing S2's inarcs forward
+     * to S3.  If we include the arcs already copied from S1 in that, we'd be
+     * doing O(N^3) work.  (The duplicate-arc elimination built into newarc()
+     * and its cohorts would get rid of the extra arcs, but not without cost.)
+     *
+     * We can avoid this cost by treating only arcs that existed at the start
+     * of this phase as candidates to be pushed forward.  To identify those,
+     * we remember the first inarc each state had to start with.  We rely on
+     * the fact that newarc() and friends put new arcs on the front of their
+     * to-states' inchains, and that this phase never deletes arcs, so that
+     * the original arcs must be the last arcs in their to-states' inchains.
      *
-     * If we just do this straightforwardly, the algorithm gets slow in
-     * complex graphs, because the same arcs get copied to all
-     * intermediate states of an EMPTY chain, and then uselessly pushed
-     * repeatedly to the chain's final state; we waste a lot of time in
-     * newarc's duplicate checking.  To improve matters, we decree that
-     * any state with only EMPTY out-arcs is "doomed" and will not be
-     * part of the final NFA. That can be ensured by not adding any new
-     * out-arcs to such a state. Having ensured that, we need not update
-     * the state's in-arcs list either; all arcs that might have gotten
-     * pushed forward to it will just get pushed directly to successor
-     * states.  This eliminates most of the useless duplicate arcs.
+     * So the process here is that, for each state in the NFA, we gather up
+     * all non-EMPTY inarcs of states that can reach the target state via
+     * EMPTY arcs.  We then sort, de-duplicate, and merge these arcs into the
+     * target state's inchain.  (We can safely use sort-merge for this as long
+     * as we update each state's original-arcs pointer after we add arcs to
+     * it; the sort step of mergeins probably changed the order of the old
+     * arcs.)
+     *
+     * Another refinement worth making is that, because we only add non-EMPTY
+     * arcs during this phase, and all added arcs have the same from-state as
+     * the non-EMPTY arc they were cloned from, we know ahead of time that any
+     * states having only EMPTY outarcs will be useless for lack of outarcs
+     * after we drop the EMPTY arcs.  (They cannot gain non-EMPTY outarcs if
+     * they had none to start with.)  So we need not bother to update the
+     * inchains of such states at all.
+     */
+
+    /* Remember the states' first original inarcs */
+    /* ... and while at it, count how many old inarcs there are altogether */
+    inarcsorig = (struct arc **) MALLOC(nfa->nstates * sizeof(struct arc *));
+    if (inarcsorig == NULL) {
+	NERR(REG_ESPACE);
+	return;
+    }
+    totalinarcs = 0;
+    for (s = nfa->states; s != NULL; s = s->next) {
+	inarcsorig[s->no] = s->ins;
+	totalinarcs += s->nins;
+    }
+
+    /*
+     * Create a workspace for accumulating the inarcs to be added to the
+     * current target state.  totalinarcs is probably a considerable
+     * overestimate of the space needed, but the NFA is unlikely to be large
+     * enough at this point to make it worth being smarter.
      */
+    arcarray = (struct arc **) MALLOC(totalinarcs * sizeof(struct arc *));
+    if (arcarray == NULL) {
+	NERR(REG_ESPACE);
+	FREE(inarcsorig);
+	return;
+    }
+
+    /* And iterate over the target states */
     for (s = nfa->states; s != NULL && !NISERR(); s = s->next) {
-	for (s2 = emptyreachable(s, s); s2 != s && !NISERR();
-		s2 = nexts) {
-	    /*
-	     * If s2 is doomed, we decide that (1) we will always push
-	     * arcs forward to it, not pull them back to s; and (2) we
-	     * can optimize away the push-forward, per comment above.
-	     * So do nothing.
-	     */
-	    if (s2->flag || hasnonemptyout(s2)) {
-		replaceempty(nfa, s, s2);
-	    }
+	/* Ignore target states without non-EMPTY outarcs, per note above */
+	if (!s->flag && !hasnonemptyout(s)) {
+	    continue;
+	}
 
-	    /* Reset the tmp fields as we walk back */
-	    nexts = s2->tmp;
-	    s2->tmp = NULL;
+	/* Find predecessor states and accumulate their original inarcs */
+	arccount = 0;
+	for (s2 = emptyreachable(nfa, s, s, inarcsorig); s2 != s; s2 = nexts) {
+	    /* Add s2's original inarcs to arcarray[], but ignore empties */
+	    for (a = inarcsorig[s2->no]; a != NULL; a = a->inchain) {
+		if (a->type != EMPTY) {
+		    arcarray[arccount++] = a;
+		}
+	    }
+  
+  	    /* Reset the tmp fields as we walk back */
+  	    nexts = s2->tmp;
+  	    s2->tmp = NULL;
+  	}
+  	s->tmp = NULL;
+	assert(arccount <= totalinarcs);
+
+	/* Remember how many original inarcs this state has */
+	prevnins = s->nins;
+
+	/* Add non-duplicate inarcs to target state */
+	mergeins(nfa, s, arcarray, arccount);
+
+	/* Now we must update the state's inarcsorig pointer */
+	nskip = s->nins - prevnins;
+	a = s->ins;
+	while (nskip-- > 0) {
+	    a = a->inchain;
 	}
-	s->tmp = NULL;
+	inarcsorig[s->no] = a;
     }
+  
+    FREE(arcarray);
+    FREE(inarcsorig);
+
     if (NISERR()) {
 	return;
     }
@@ -1432,89 +2081,604 @@ fixempties(
 }
 
 /*
- - emptyreachable - recursively find all states reachable from s by EMPTY arcs
+ - emptyreachable - recursively find all states that can reach s by EMPTY arcs
  * The return value is the last such state found.  Its tmp field links back
  * to the next-to-last such state, and so on back to s, so that all these
  * states can be located without searching the whole NFA.
+ *
+ * Since this is only used in fixempties(), we pass in the inarcsorig[] array
+ * maintained by that function.  This lets us skip over all new inarcs, which
+ * are certainly not EMPTY arcs.
+ *
  * The maximum recursion depth here is equal to the length of the longest
  * loop-free chain of EMPTY arcs, which is surely no more than the size of
- * the NFA, and in practice will be a lot less than that.
+ * the NFA, and in practice will be less than that.
  ^ static struct state *emptyreachable(struct state *, struct state *);
  */
 static struct state *
 emptyreachable(
+    struct nfa *nfa,
     struct state *s,
-    struct state *lastfound)
+    struct state *lastfound,
+    struct arc **inarcsorig)
 {
     struct arc *a;
 
     s->tmp = lastfound;
     lastfound = s;
-    for (a = s->outs; a != NULL; a = a->outchain) {
-	if (a->type == EMPTY && a->to->tmp == NULL) {
-	    lastfound = emptyreachable(a->to, lastfound);
+    for (a = inarcsorig[s->no]; a != NULL; a = a->inchain) {
+	if (a->type == EMPTY && a->from->tmp == NULL) {
+	    lastfound = emptyreachable(nfa, a->from, lastfound, inarcsorig);
 	}
     }
     return lastfound;
 }
-
+
+/*
+ * isconstraintarc - detect whether an arc is of a constraint type
+ */
+static inline int
+isconstraintarc(struct arc * a)
+{
+    switch (a->type)
+    {
+	case '^':
+	case '$':
+	case BEHIND:
+	case AHEAD:
+	case LACON:
+	    return 1;
+    }
+    return 0;
+}
+
 /*
- - replaceempty - replace an EMPTY arc chain with some non-empty arcs
- * The EMPTY arc(s) should be deleted later, but we can't do it here because
- * they may still be needed to identify other arc chains during fixempties().
- ^ static void replaceempty(struct nfa *, struct state *, struct state *);
+ * hasconstraintout - does state have a constraint out arc?
+ */
+static int
+hasconstraintout(struct state * s)
+{
+    struct arc *a;
+
+    for (a = s->outs; a != NULL; a = a->outchain) {
+	if (isconstraintarc(a)) {
+	    return 1;
+	}
+    }
+    return 0;
+}
+
+/*
+ * fixconstraintloops - get rid of loops containing only constraint arcs
+ *
+ * A loop of states that contains only constraint arcs is useless, since
+ * passing around the loop represents no forward progress.  Moreover, it
+ * would cause infinite looping in pullback/pushfwd, so we need to get rid
+ * of such loops before doing that.
  */
 static void
-replaceempty(
-    struct nfa *nfa,
-    struct state *from,
-    struct state *to)
+fixconstraintloops(
+    struct nfa * nfa,
+    FILE *f)		/* for debug output; NULL none */
 {
-    int fromouts;
-    int toins;
+    struct state *s;
+    struct state *nexts;
+    struct arc *a;
+    struct arc *nexta;
+    int hasconstraints;
+
+    /*
+     * In the trivial case of a state that loops to itself, we can just drop
+     * the constraint arc altogether.  This is worth special-casing because
+     * such loops are far more common than loops containing multiple states.
+     * While we're at it, note whether any constraint arcs survive.
+     */
+    hasconstraints = 0;
+    for (s = nfa->states; s != NULL && !NISERR(); s = nexts) {
+	nexts = s->next;
+	/* while we're at it, ensure tmp fields are clear for next step */
+	assert(s->tmp == NULL);
+	for (a = s->outs; a != NULL && !NISERR(); a = nexta) {
+	    nexta = a->outchain;
+	    if (isconstraintarc(a)) {
+		if (a->to == s) {
+		    freearc(nfa, a);
+		} else {
+		    hasconstraints = 1;
+ 		}
+	    }
+	}
+ 	/* If we removed all the outarcs, the state is useless. */
+ 	if (s->nouts == 0 && !s->flag) {
+ 	    dropstate(nfa, s);
+	}
+    }
+ 
+    /* Nothing to do if no remaining constraint arcs */
+    if (NISERR() || !hasconstraints) {
+	return;
+    }
+
+    /*
+     * Starting from each remaining NFA state, search outwards for a
+     * constraint loop.  If we find a loop, break the loop, then start the
+     * search over.  (We could possibly retain some state from the first scan,
+     * but it would complicate things greatly, and multi-state constraint
+     * loops are rare enough that it's not worth optimizing the case.)
+     */
+  restart:
+    for (s = nfa->states; s != NULL && !NISERR(); s = s->next) {
+	if (findconstraintloop(nfa, s)) {
+	    goto restart;
+	}
+    }
 
-    assert(from != to);
+    if (NISERR()) {
+	return;
+    }
 
     /*
-     * Create replacement arcs that bypass the need for the EMPTY chain.  We
-     * can do this either by pushing arcs forward (linking directly from
-     * "from"'s predecessors to "to") or by pulling them back (linking
-     * directly from "from" to "to"'s successors).  In general, we choose
-     * whichever way creates greater fan-out or fan-in, so as to improve the
-     * odds of reducing the other state to zero in-arcs or out-arcs and
-     * thereby being able to delete it.  However, if "from" is doomed (has no
-     * non-EMPTY out-arcs), we must keep it so, so always push forward in that
-     * case.
+     * Now remove any states that have become useless.  (This cleanup is not
+     * very thorough, and would be even less so if we tried to combine it with
+     * the previous step; but cleanup() will take care of anything we miss.)
      *
-     * The fan-out/fan-in comparison should count only non-EMPTY arcs.  If
-     * "from" is doomed, we can skip counting "to"'s arcs, since we want to
-     * force taking the copynonemptyins path in that case.
+     * Because findconstraintloop intentionally doesn't reset all tmp fields,
+     * we have to clear them after it's done.  This is a convenient place to
+     * do that, too.
+     */
+    for (s = nfa->states; s != NULL; s = nexts) {
+	nexts = s->next;
+	s->tmp = NULL;
+	if ((s->nins == 0 || s->nouts == 0) && !s->flag) {
+	    dropstate(nfa, s);
+	}
+    }
+
+    if (f != NULL) {
+ 	dumpnfa(nfa, f);
+    }
+}
+
+/*
+ * findconstraintloop - recursively find a loop of constraint arcs
+ *
+ * If we find a loop, break it by calling breakconstraintloop(), then
+ * return 1; otherwise return 0.
+ *
+ * State tmp fields are guaranteed all NULL on a success return, because
+ * breakconstraintloop does that.  After a failure return, any state that
+ * is known not to be part of a loop is marked with s->tmp == s; this allows
+ * us not to have to re-prove that fact on later calls.  (This convention is
+ * workable because we already eliminated single-state loops.)
+ *
+ * Note that the found loop doesn't necessarily include the first state we
+ * are called on.  Any loop reachable from that state will do.
+ *
+ * The maximum recursion depth here is one more than the length of the longest
+ * loop-free chain of constraint arcs, which is surely no more than the size
+ * of the NFA, and in practice will be a lot less than that.
+ */
+static int
+findconstraintloop(struct nfa * nfa, struct state * s)
+{
+    struct arc *a;
+
+    /* Since this is recursive, it could be driven to stack overflow */
+    if (STACK_TOO_DEEP(nfa->v->re)) {
+	NERR(REG_ETOOBIG);
+	return 1;		/* to exit as quickly as possible */
+    }
+
+    if (s->tmp != NULL) {
+	/* Already proven uninteresting? */
+	if (s->tmp == s) {
+	    return 0;
+	}
+	/* Found a loop involving s */
+	breakconstraintloop(nfa, s);
+	/* The tmp fields have been cleaned up by breakconstraintloop */
+	return 1;
+    }
+    for (a = s->outs; a != NULL; a = a->outchain) {
+	if (isconstraintarc(a)) {
+	    struct state *sto = a->to;
+
+	    assert(sto != s);
+	    s->tmp = sto;
+	    if (findconstraintloop(nfa, sto)) {
+		return 1;
+	    }
+	}
+    }
+
+    /*
+     * If we get here, no constraint loop exists leading out from s.  Mark it
+     * with s->tmp == s so we need not rediscover that fact again later.
+     */
+    s->tmp = s;
+    return 0;
+}
+
+/*
+ * breakconstraintloop - break a loop of constraint arcs
+ *
+ * sinitial is any one member state of the loop.  Each loop member's tmp
+ * field links to its successor within the loop.  (Note that this function
+ * will reset all the tmp fields to NULL.)
+ *
+ * We can break the loop by, for any one state S1 in the loop, cloning its
+ * loop successor state S2 (and possibly following states), and then moving
+ * all S1->S2 constraint arcs to point to the cloned S2.  The cloned S2 should
+ * copy any non-constraint outarcs of S2.  Constraint outarcs should be
+ * dropped if they point back to S1, else they need to be copied as arcs to
+ * similarly cloned states S3, S4, etc.  In general, each cloned state copies
+ * non-constraint outarcs, drops constraint outarcs that would lead to itself
+ * or any earlier cloned state, and sends other constraint outarcs to newly
+ * cloned states.  No cloned state will have any inarcs that aren't constraint
+ * arcs or do not lead from S1 or earlier-cloned states.  It's okay to drop
+ * constraint back-arcs since they would not take us to any state we've not
+ * already been in; therefore, no new constraint loop is created.  In this way
+ * we generate a modified NFA that can still represent every useful state
+ * sequence, but not sequences that represent state loops with no consumption
+ * of input data.  Note that the set of cloned states will certainly include
+ * all of the loop member states other than S1, and it may also include
+ * non-loop states that are reachable from S2 via constraint arcs.  This is
+ * important because there is no guarantee that findconstraintloop found a
+ * maximal loop (and searching for one would be NP-hard, so don't try).
+ * Frequently the "non-loop states" are actually part of a larger loop that
+ * we didn't notice, and indeed there may be several overlapping loops.
+ * This technique ensures convergence in such cases, while considering only
+ * the originally-found loop does not.
+ *
+ * If there is only one S1->S2 constraint arc, then that constraint is
+ * certainly satisfied when we enter any of the clone states.  This means that
+ * in the common case where many of the constraint arcs are identically
+ * labeled, we can merge together clone states linked by a similarly-labeled
+ * constraint: if we can get to the first one we can certainly get to the
+ * second, so there's no need to distinguish.  This greatly reduces the number
+ * of new states needed, so we preferentially break the given loop at a state
+ * pair where this is true.
+ *
+ * Furthermore, it's fairly common to find that a cloned successor state has
+ * no outarcs, especially if we're a bit aggressive about removing unnecessary
+ * outarcs.  If that happens, then there is simply not any interesting state
+ * that can be reached through the predecessor's loop arcs, which means we can
+ * break the loop just by removing those loop arcs, with no new states added.
+ */
+static void
+breakconstraintloop(struct nfa * nfa, struct state * sinitial)
+{
+    struct state *s;
+    struct state *shead;
+    struct state *stail;
+    struct state *sclone;
+    struct state *nexts;
+    struct arc *refarc;
+    struct arc *a;
+    struct arc *nexta;
+
+    /*
+     * Start by identifying which loop step we want to break at.
+     * Preferentially this is one with only one constraint arc.  (XXX are
+     * there any other secondary heuristics we want to use here?)  Set refarc
+     * to point to the selected lone constraint arc, if there is one.
+     */
+    refarc = NULL;
+    s = sinitial;
+    do {
+	nexts = s->tmp;
+	assert(nexts != s);	/* should not see any one-element loops */
+	if (refarc == NULL) {
+	    int narcs = 0;
+
+	    for (a = s->outs; a != NULL; a = a->outchain) {
+		if (a->to == nexts && isconstraintarc(a)) {
+		    refarc = a;
+		    narcs++;
+		}
+	    }
+	    assert(narcs > 0);
+	    if (narcs > 1) {
+		refarc = NULL;	/* multiple constraint arcs here, no good */
+	    }
+	}
+	s = nexts;
+    } while (s != sinitial);
+
+    if (refarc) {
+	/* break at the refarc */
+	shead = refarc->from;
+	stail = refarc->to;
+	assert(stail == shead->tmp);
+    } else {
+	/* for lack of a better idea, break after sinitial */
+	shead = sinitial;
+	stail = sinitial->tmp;
+    }
+
+    /*
+     * Reset the tmp fields so that we can use them for local storage in
+     * clonesuccessorstates.  (findconstraintloop won't mind, since it's just
+     * going to abandon its search anyway.)
      */
-    fromouts = nonemptyouts(from);
-    toins = (fromouts == 0) ? 1 : nonemptyins(to);
+    for (s = nfa->states; s != NULL; s = s->next) {
+	s->tmp = NULL;
+    }
 
-    if (fromouts > toins) {
-	copyouts(nfa, to, from, 0);
+    /*
+     * Recursively build clone state(s) as needed.
+     */
+    sclone = newstate(nfa);
+    if (sclone == NULL) {
+	assert(NISERR());
 	return;
     }
-    if (fromouts < toins) {
-	copyins(nfa, from, to, 0);
+
+    clonesuccessorstates(nfa, stail, sclone, shead, refarc,
+	    NULL, NULL, nfa->nstates);
+
+    if (NISERR()) {
 	return;
     }
 
     /*
-     * fromouts == toins.  Decide on secondary issue: copy fewest arcs.
-     *
-     * Doesn't seem to be worth the trouble to exclude empties from these
-     * comparisons; that takes extra time and doesn't seem to improve the
-     * resulting graph much.
+     * It's possible that sclone has no outarcs at all, in which case it's
+     * useless.  (We don't try extremely hard to get rid of useless states
+     * here, but this is an easy and fairly common case.)
      */
-    if (from->nins > to->nouts) {
-	copyouts(nfa, to, from, 0);
+    if (sclone->nouts == 0) {
+	freestate(nfa, sclone);
+	sclone = NULL;
+    }
+
+    /*
+     * Move shead's constraint-loop arcs to point to sclone, or just drop them
+     * if we discovered we don't need sclone.
+     */
+    for (a = shead->outs; a != NULL; a = nexta) {
+	nexta = a->outchain;
+	if (a->to == stail && isconstraintarc(a)) {
+	    if (sclone) {
+		cparc(nfa, a, shead, sclone);
+	    }
+	    freearc(nfa, a);
+	    if (NISERR()) {
+		break;
+	    }
+	}
+    }
+}
+
+/*
+ * clonesuccessorstates - create a tree of constraint-arc successor states
+ *
+ * ssource is the state to be cloned, and sclone is the state to copy its
+ * outarcs into.  sclone's inarcs, if any, should already be set up.
+ *
+ * spredecessor is the original predecessor state that we are trying to build
+ * successors for (it may not be the immediate predecessor of ssource).
+ * refarc, if not NULL, is the original constraint arc that is known to have
+ * been traversed out of spredecessor to reach the successor(s).
+ *
+ * For each cloned successor state, we transiently create a "donemap" that is
+ * a boolean array showing which source states we've already visited for this
+ * clone state.  This prevents infinite recursion as well as useless repeat
+ * visits to the same state subtree (which can add up fast, since typical NFAs
+ * have multiple redundant arc pathways).  Each donemap is a char array
+ * indexed by state number.  The donemaps are all of the same size "nstates",
+ * which is nfa->nstates as of the start of the recursion.  This is enough to
+ * have entries for all pre-existing states, but *not* entries for clone
+ * states created during the recursion.  That's okay since we have no need to
+ * mark those.
+ *
+ * curdonemap is NULL when recursing to a new sclone state, or sclone's
+ * donemap when we are recursing without having created a new state (which we
+ * do when we decide we can merge a successor state into the current clone
+ * state).  outerdonemap is NULL at the top level and otherwise the parent
+ * clone state's donemap.
+ *
+ * The successor states we create and fill here form a strict tree structure,
+ * with each state having exactly one predecessor, except that the toplevel
+ * state has no inarcs as yet (breakconstraintloop will add its inarcs from
+ * spredecessor after we're done).  Thus, we can examine sclone's inarcs back
+ * to the root, plus refarc if any, to identify the set of constraints already
+ * known valid at the current point.  This allows us to avoid generating extra
+ * successor states.
+ */
+static void
+clonesuccessorstates(
+    struct nfa * nfa,
+    struct state * ssource,
+    struct state * sclone,
+    struct state * spredecessor,
+    struct arc * refarc,
+    char *curdonemap,
+    char *outerdonemap,
+    int nstates)
+{
+    char *donemap;
+    struct arc *a;
+
+    /* Since this is recursive, it could be driven to stack overflow */
+    if (STACK_TOO_DEEP(nfa->v->re)) {
+	NERR(REG_ETOOBIG);
 	return;
     }
 
-    copyins(nfa, from, to, 0);
+    /* If this state hasn't already got a donemap, create one */
+    donemap = curdonemap;
+    if (donemap == NULL) {
+	donemap = (char *) MALLOC(nstates * sizeof(char));
+	if (donemap == NULL) {
+	    NERR(REG_ESPACE);
+	    return;
+	}
+
+	if (outerdonemap != NULL) {
+	    /*
+	     * Not at outermost recursion level, so copy the outer level's
+	     * donemap; this ensures that we see states in process of being
+	     * visited at outer levels, or already merged into predecessor
+	     * states, as ones we shouldn't traverse back to.
+	     */
+	    memcpy(donemap, outerdonemap, nstates * sizeof(char));
+	} else {
+	    /* At outermost level, only spredecessor is off-limits */
+	    memset(donemap, 0, nstates * sizeof(char));
+	    assert(spredecessor->no < nstates);
+	    donemap[spredecessor->no] = 1;
+	}
+    }
+
+    /* Mark ssource as visited in the donemap */
+    assert(ssource->no < nstates);
+    assert(donemap[ssource->no] == 0);
+    donemap[ssource->no] = 1;
+
+    /*
+     * We proceed by first cloning all of ssource's outarcs, creating new
+     * clone states as needed but not doing more with them than that.  Then in
+     * a second pass, recurse to process the child clone states.  This allows
+     * us to have only one child clone state per reachable source state, even
+     * when there are multiple outarcs leading to the same state.  Also, when
+     * we do visit a child state, its set of inarcs is known exactly, which
+     * makes it safe to apply the constraint-is-already-checked optimization.
+     * Also, this ensures that we've merged all the states we can into the
+     * current clone before we recurse to any children, thus possibly saving
+     * them from making extra images of those states.
+     *
+     * While this function runs, child clone states of the current state are
+     * marked by setting their tmp fields to point to the original state they
+     * were cloned from.  This makes it possible to detect multiple outarcs
+     * leading to the same state, and also makes it easy to distinguish clone
+     * states from original states (which will have tmp == NULL).
+     */
+    for (a = ssource->outs; a != NULL && !NISERR(); a = a->outchain) {
+	struct state *sto = a->to;
+
+	/*
+	 * We do not consider cloning successor states that have no constraint
+	 * outarcs; just link to them as-is.  They cannot be part of a
+	 * constraint loop so there is no need to make copies.  In particular,
+	 * this rule keeps us from trying to clone the post state, which would
+	 * be a bad idea.
+	 */
+	if (isconstraintarc(a) && hasconstraintout(sto)) {
+	    struct state *prevclone;
+	    int canmerge;
+	    struct arc *a2;
+
+	    /*
+	     * Back-link constraint arcs must not be followed.  Nor is there a
+	     * need to revisit states previously merged into this clone.
+	     */
+	    assert(sto->no < nstates);
+	    if (donemap[sto->no] != 0) {
+		continue;
+	    }
+
+	    /*
+	     * Check whether we already have a child clone state for this
+	     * source state.
+	     */
+	    prevclone = NULL;
+	    for (a2 = sclone->outs; a2 != NULL; a2 = a2->outchain) {
+		if (a2->to->tmp == sto) {
+		    prevclone = a2->to;
+		    break;
+		}
+	    }
+
+	    /*
+	     * If this arc is labeled the same as refarc, or the same as any
+	     * arc we must have traversed to get to sclone, then no additional
+	     * constraints need to be met to get to sto, so we should just
+	     * merge its outarcs into sclone.
+	     */
+	    if (refarc && a->type == refarc->type && a->co == refarc->co) {
+		canmerge = 1;
+	    } else {
+		struct state *s;
+
+		canmerge = 0;
+		for (s = sclone; s->ins; s = s->ins->from) {
+		    if (s->nins == 1 &&
+			    a->type == s->ins->type && a->co == s->ins->co) {
+			canmerge = 1;
+			break;
+		    }
+		}
+	    }
+
+	    if (canmerge) {
+		/*
+		 * We can merge into sclone.  If we previously made a child
+		 * clone state, drop it; there's no need to visit it.  (This
+		 * can happen if ssource has multiple pathways to sto, and we
+		 * only just now found one that is provably a no-op.)
+		 */
+		if (prevclone) {
+		    dropstate(nfa, prevclone);	/* kills our outarc, too */
+		}
+
+		/* Recurse to merge sto's outarcs into sclone */
+		clonesuccessorstates(nfa, sto, sclone, spredecessor, refarc,
+			donemap, outerdonemap, nstates);
+		/* sto should now be marked as previously visited */
+		assert(NISERR() || donemap[sto->no] == 1);
+	    } else if (prevclone) {
+		/*
+		 * We already have a clone state for this successor, so just
+		 * make another arc to it.
+		 */
+		cparc(nfa, a, sclone, prevclone);
+	    } else {
+		/*
+		 * We need to create a new successor clone state.
+		 */
+		struct state *stoclone;
+
+		stoclone = newstate(nfa);
+		if (stoclone == NULL) {
+		    assert(NISERR());
+		    break;
+		}
+		/* Mark it as to what it's a clone of */
+		stoclone->tmp = sto;
+		/* ... and add the outarc leading to it */
+		cparc(nfa, a, sclone, stoclone);
+	    }
+	} else {
+	    /*
+	     * Non-constraint outarcs just get copied to sclone, as do outarcs
+	     * leading to states with no constraint outarc.
+	     */
+	    cparc(nfa, a, sclone, sto);
+	}
+    }
+
+    /*
+     * If we are at outer level for this clone state, recurse to all its child
+     * clone states, clearing their tmp fields as we go.  (If we're not
+     * outermost for sclone, leave this to be done by the outer call level.)
+     * Note that if we have multiple outarcs leading to the same clone state,
+     * it will only be recursed-to once.
+     */
+    if (curdonemap == NULL) {
+	for (a = sclone->outs; a != NULL && !NISERR(); a = a->outchain) {
+	    struct state *stoclone = a->to;
+	    struct state *sto = stoclone->tmp;
+
+	    if (sto != NULL) {
+		stoclone->tmp = NULL;
+		clonesuccessorstates(nfa, sto, stoclone, spredecessor, refarc,
+			NULL, donemap, nstates);
+	    }
+	}
+
+	/* Don't forget to free sclone's donemap when done with it */
+	FREE(donemap);
+    }
 }
 
 /*
@@ -1631,7 +2795,7 @@ analyze(
 }
 
 /*
- - compact - compact an NFA
+ - compact - construct the compact representation of an NFA
  ^ static void compact(struct nfa *, struct cnfa *);
  */
 static void
@@ -1702,11 +2866,11 @@ compact(
 		cnfa->flags |= HASLACONS;
 		break;
 	    default:
-		assert(NOTREACHED);
+		NERR(REG_ASSERT);
 		break;
 	    }
 	}
-	carcsort(first, ca-1);
+	carcsort(first, ca - first);
 	ca->co = COLORLESS;
 	ca->to = 0;
 	ca++;
@@ -1726,33 +2890,39 @@ compact(
 
 /*
  - carcsort - sort compacted-NFA arcs by color
- * Really dumb algorithm, but if the list is long enough for that to matter,
- * you're in real trouble anyway.
  ^ static void carcsort(struct carc *, struct carc *);
  */
 static void
 carcsort(
     struct carc *first,
-    struct carc *last)
+    size_t n)
 {
-    struct carc *p;
-    struct carc *q;
-    struct carc tmp;
-
-    if (last - first <= 1) {
-	return;
+    if (n > 1) {
+	qsort(first, n, sizeof(struct carc), carc_cmp);
     }
+}
 
-    for (p = first; p <= last; p++) {
-	for (q = p; q <= last; q++) {
-	    if (p->co > q->co || (p->co == q->co && p->to > q->to)) {
-		assert(p != q);
-		tmp = *p;
-		*p = *q;
-		*q = tmp;
-	    }
-	}
+static int
+carc_cmp(
+    const void *a,
+    const void *b)
+{
+    const struct carc *aa = (const struct carc *) a;
+    const struct carc *bb = (const struct carc *) b;
+  
+    if (aa->co < bb->co) {
+	return -1;
+    }
+    if (aa->co > bb->co) {
+	return +1;
+    }
+    if (aa->to < bb->to) {
+	return -1;
+    }
+    if (aa->to > bb->to) {
+	return +1;
     }
+    return 0;
 }
 
 /*
@@ -1781,6 +2951,8 @@ dumpnfa(
 {
 #ifdef REG_DEBUG
     struct state *s;
+    int nstates = 0;
+    int narcs = 0;
 
     fprintf(f, "pre %d, post %d", nfa->pre->no, nfa->post->no);
     if (nfa->bos[0] != COLORLESS) {
@@ -1798,7 +2970,10 @@ dumpnfa(
     fprintf(f, "\n");
     for (s = nfa->states; s != NULL; s = s->next) {
 	dumpstate(s, f);
+	nstates++;
+	narcs += s->nouts;
     }
+    fprintf(f, "total of %d states, %d arcs\n", nstates, narcs);
     if (nfa->parent == NULL) {
 	dumpcolors(nfa->cm, f);
     }
@@ -1851,37 +3026,28 @@ dumparcs(
     FILE *f)
 {
     int pos;
+    struct arc *a;
 
-    assert(s->nouts > 0);
-    /* printing arcs in reverse order is usually clearer */
-    pos = dumprarcs(s->outs, s, f, 1);
-    if (pos != 1) {
-	fprintf(f, "\n");
-    }
-}
-
-/*
- - dumprarcs - dump remaining outarcs, recursively, in reverse order
- ^ static int dumprarcs(struct arc *, struct state *, FILE *, int);
- */
-static int			/* resulting print position */
-dumprarcs(
-    struct arc *a,
-    struct state *s,
-    FILE *f,
-    int pos)			/* initial print position */
-{
-    if (a->outchain != NULL) {
-	pos = dumprarcs(a->outchain, s, f, pos);
+    /* printing oldest arcs first is usually clearer */
+    a = s->outs;
+    assert(a != NULL);
+    while (a->outchain != NULL) {
+	a = a->outchain;
     }
-    dumparc(a, s, f);
-    if (pos == 5) {
+    pos = 1;
+    do {
+	dumparc(a, s, f);
+	if (pos == 5) {
+	    fprintf(f, "\n");
+	    pos = 1;
+	} else {
+	    pos++;
+	}
+	a = a->outchainRev;
+    } while (a != NULL);
+    if (pos != 1) {
 	fprintf(f, "\n");
-	pos = 1;
-    } else {
-	pos++;
     }
-    return pos;
 }
 
 /*