made DOTGen nonrecursive; add DFA verification stuff

[git-p4: depot-paths = "//depot/code/antlr4/main/": change = 6749]

tool/resources/org/antlr/v4/tool/templates/dot/state.st

@@ -1,3 +1,3 @@
 state(state, name) ::= <<
-node [fontsize=11, <if(useBox)>shape=box, fixedsize=false<else>shape=circle, fixedsize=true, width=.4<endif>]; <name>
+node [fontsize=11, label="<label>", <if(useBox)>shape=box, fixedsize=false<else>shape=circle, fixedsize=true, width=.4<endif>, peripheries=1]; <name>
 >>

tool/resources/org/antlr/v4/tool/templates/dot/stopstate.st

@@ -1,3 +1,3 @@
 stopstate(name) ::= <<
-node [fontsize=11, <if(useBox)>shape=polygon,sides=4,peripheries=2,fixedsize=false<else>shape=doublecircle, fixedsize=true, width=.6<endif>]; <name>
+node [fontsize=11, label="<label>", <if(useBox)>shape=polygon,sides=4,peripheries=2,fixedsize=false<else>shape=doublecircle, fixedsize=true, width=.6<endif>]; <name>
 >>

tool/src/org/antlr/v4/analysis/DFAVerifier.java

@@ -1,9 +1,12 @@
 package org.antlr.v4.analysis;
 
 import org.antlr.v4.automata.DFA;
+import org.antlr.v4.automata.DFAState;
+import org.antlr.v4.automata.Edge;
+import org.antlr.v4.misc.Utils;
+import org.stringtemplate.v4.misc.MultiMap;
 
-import java.util.HashSet;
+import java.util.*;
-import java.util.Set;
 
 /** Detect imperfect DFA:
  *
@@ -12,7 +15,17 @@ import java.util.Set;
  *  3. nondeterministic states
  */
 public class DFAVerifier {
+	public static enum ReachableStatus {
+		UNKNOWN,
+		BUSY, // in process of computing
+		NO,
+		YES;
+	}
+	
+	Map<DFAState, ReachableStatus> status = new HashMap<DFAState, ReachableStatus>();
+	
 	DFA dfa;
+
 	StackLimitedNFAToDFAConverter converter;
 
 	public DFAVerifier(DFA dfa, StackLimitedNFAToDFAConverter converter) {
@@ -20,11 +33,128 @@ public class DFAVerifier {
 		this.converter = converter;
 	}
 
-	public void analyze() {
-
-	}
-
 	public Set<Integer> getUnreachableAlts() {
-		return new HashSet<Integer>();
+		Set<Integer> unreachable = new HashSet<Integer>();
+		for (int alt=0; alt<dfa.nAlts; alt++) {
+			if ( dfa.altToAcceptState[alt]==null ) unreachable.add(alt);
+		}
+		return unreachable; 
+	}
+
+	public Set<DFAState> getDeadStates() {
+		// create 2D matrix showing incident edges; inverse of adjacency matrix 
+		// incidentEdges.get(s) is list of edges pointing at state s 
+		MultiMap<DFAState, DFAState> incidentStates = new MultiMap<DFAState, DFAState>();
+		for (DFAState d : dfa.states.values()) {
+			for (Edge e : d.edges) incidentStates.map(e.target, d);
+		}
+		//Set<DFAState> reaches = new HashSet<DFAState>(dfa.uniqueStates.size());
+
+		Set<DFAState> dead = new HashSet<DFAState>(dfa.states.size());
+		dead.addAll(dfa.states.values());
+		for (DFAState a : dfa.altToAcceptState) {
+			if ( a!=null ) dead.remove(a);
+		}
+
+		// obviously accept states reach accept states
+		//reaches.addAll(Arrays.asList(dfa.altToAcceptState));
+
+		boolean changed = true;
+		while ( changed ) {
+			changed = false;
+			for (DFAState d : dfa.states.values()) {
+				if ( !dead.contains(d) ) {
+					// if d isn't dead, it reaches accept state.
+					dead.remove(d);
+					// and, so do all states pointing at it
+					List<DFAState> incoming = incidentStates.get(d);
+					if ( incoming!=null ) dead.removeAll(incoming);
+					changed = true;
+				}
+			}
+		}
+
+//		boolean changed = true;
+//		while ( changed ) {
+//			changed = false;
+//			for (DFAState d : dfa.uniqueStates.values()) {
+//				if ( reaches.contains(d) ) {
+//					dead.remove(d);
+//					// if d reaches, so do all states pointing at it
+//					for (DFAState i : incidentStates.get(d)) {
+//						if ( !reaches.contains(i) ) {
+//							reaches.add(i);
+//							changed = true;
+//						}
+//					}
+//				}
+//			}
+//		}
+
+		System.out.println("dead="+dead);
+		return dead;
+	}
+
+	/** figure out if this state eventually reaches an accept state and
+	 *  modify the instance variable 'reduced' to indicate if we find
+	 *  at least one state that cannot reach an accept state.  This implies
+	 *  that the overall DFA is not reduced.  This algorithm should be
+	 *  linear in the number of DFA states.
+	 *
+	 *  The algorithm also tracks which alternatives have no accept state,
+	 *  indicating a nondeterminism.
+	 *
+	 *  Also computes whether the DFA is cyclic.
+	 *
+	 *  TODO: I call getUniquelyPredicatedAlt too much; cache predicted alt
+	 *  TODO: convert to nonrecursive version.
+	 */
+	 boolean _verify(DFAState d) { // TODO: rename to verify?
+		if ( d.isAcceptState ) {
+			// accept states have no edges emanating from them so we can return
+			status.put(d, ReachableStatus.YES);
+			// this alt is uniquely predicted, remove from nondeterministic list
+			int predicts = d.getUniquelyPredictedAlt();
+			converter.unreachableAlts.remove(Utils.integer(predicts));
+			return true;
+		}
+
+		// avoid infinite loops
+		status.put(d, ReachableStatus.BUSY);
+
+		boolean anEdgeReachesAcceptState = false;
+		// Visit every transition, track if at least one edge reaches stop state
+		// Cannot terminate when we know this state reaches stop state since
+		// all transitions must be traversed to set status of each DFA state.
+		for (int i=0; i<d.getNumberOfTransitions(); i++) {
+			Edge t = d.transition(i);
+			DFAState edgeTarget = (DFAState)t.target;
+			ReachableStatus targetStatus = status.get(edgeTarget);
+			if ( targetStatus==ReachableStatus.BUSY ) { // avoid cycles; they say nothing
+				converter.cyclic = true;
+				continue;
+			}
+			if ( targetStatus==ReachableStatus.YES ) { // avoid unnecessary work
+				anEdgeReachesAcceptState = true;
+				continue;
+			}
+			if ( targetStatus==ReachableStatus.NO ) {  // avoid unnecessary work
+				continue;
+			}
+			// target must be ReachableStatus.UNKNOWN (i.e., unvisited)
+			if ( _verify(edgeTarget) ) {
+				anEdgeReachesAcceptState = true;
+				// have to keep looking so don't break loop
+				// must cover all states even if we find a path for this state
+			}
+		}
+		if ( anEdgeReachesAcceptState ) {
+			status.put(d, ReachableStatus.YES);
+		}
+		else {
+			status.put(d, ReachableStatus.NO);
+			converter.reduced = false;
+		}
+		return anEdgeReachesAcceptState;
 	}
 }

tool/src/org/antlr/v4/analysis/Resolver.java

@@ -26,7 +26,8 @@ public class Resolver {
 	 *  conflicting ctx predicts alts i and j.  Return an Integer set
 	 *  of the alternative numbers that conflict.  Two contexts conflict if
 	 *  they are equal or one is a stack suffix of the other or one is
-	 *  the empty context.
+	 *  the empty context.  The conflict is a true ambiguity. No amount
+	 *  of further looking in grammar will resolve issue (only preds help).
 	 *
 	 *  Use a hash table to record the lists of configs for each state
 	 *  as they are encountered.  We need only consider states for which
@@ -34,9 +35,9 @@ public class Resolver {
 	 *  alt must be different or must have different contexts to avoid a
 	 *  conflict.
 	 */
-	public Set<Integer> getNonDeterministicAlts(DFAState d) {
+	public Set<Integer> getAmbiguousAlts(DFAState d) {
 		//System.out.println("getNondetAlts for DFA state "+stateNumber);
-		 Set<Integer> nondeterministicAlts = new HashSet<Integer>();
+		 Set<Integer> ambiguousAlts = new HashSet<Integer>();
 
 		// If only 1 NFA conf then no way it can be nondeterministic;
 		// save the overhead.  There are many o-a->o NFA transitions
@@ -54,7 +55,8 @@ public class Resolver {
 		}
 
 		// potential conflicts are states with > 1 configuration and diff alts
-		boolean thisStateHasPotentialProblem = false;
+//		boolean thisStateHasPotentialProblem = false;
+		boolean deterministic = true;
 		for (List<NFAConfig> configsForState : stateToConfigListMap.values()) {
 			if ( configsForState.size()>1 ) {
 				int predictedAlt = Resolver.getUniqueAlt(configsForState, false);
@@ -65,13 +67,19 @@ public class Resolver {
 					stateToConfigListMap.put(configsForState.get(0).state.stateNumber, null);
 				}
 				else {
-					thisStateHasPotentialProblem = true;
+					//thisStateHasPotentialProblem = true;
+					deterministic = false;
 				}
 			}
 		}
 
 		// a fast check for potential issues; most states have none
-		if ( !thisStateHasPotentialProblem ) return null;
+//		if ( !thisStateHasPotentialProblem ) return null;
+
+		if ( deterministic ) {
+			d.isAcceptState = true;
+			return null;
+		}
 
 		// we have a potential problem, so now go through config lists again
 		// looking for different alts (only states with potential issues
@@ -107,35 +115,34 @@ public class Resolver {
 						ctxConflict = s.context.conflictsWith(t.context);
 					}
 					if ( altConflict && ctxConflict ) {
-						nondeterministicAlts.add(s.alt);
+						ambiguousAlts.add(s.alt);
-						nondeterministicAlts.add(t.alt);
+						ambiguousAlts.add(t.alt);
 					}
 				}
 			}
 		}
 
-		if ( nondeterministicAlts.size()==0 ) return null;
+		if ( ambiguousAlts.size()==0 ) return null;
-		return nondeterministicAlts;
+		return ambiguousAlts;
 	}
 
-	public void resolveNonDeterminisms(DFAState d) {
+	public void resolveAmbiguities(DFAState d) {
 		if ( StackLimitedNFAToDFAConverter.debug ) {
 			System.out.println("resolveNonDeterminisms "+d.toString());
 		}
-		Set<Integer> nondeterministicAlts = getNonDeterministicAlts(d);
+		Set<Integer> ambiguousAlts = getAmbiguousAlts(d);
-		if ( StackLimitedNFAToDFAConverter.debug && nondeterministicAlts!=null ) {
+		if ( StackLimitedNFAToDFAConverter.debug && ambiguousAlts!=null ) {
-			System.out.println("nondet alts="+nondeterministicAlts);
+			System.out.println("ambig alts="+ambiguousAlts);
 		}
 
 		// if no problems return
-		if ( nondeterministicAlts==null ) return;
+		if ( ambiguousAlts==null ) return;
 
-		// reportNondeterminism(d, nondeterministicAlts);
+		converter.ambiguousStates.add(d);
-		converter.nondeterministicStates.add(d);
 
 		// ATTEMPT TO RESOLVE WITH SEMANTIC PREDICATES
 		boolean resolved =
-			semResolver.tryToResolveWithSemanticPredicates(d, nondeterministicAlts);
+			semResolver.tryToResolveWithSemanticPredicates(d, ambiguousAlts);
 		if ( resolved ) {
 			if ( StackLimitedNFAToDFAConverter.debug ) {
 				System.out.println("resolved DFA state "+d.stateNumber+" with pred");
@@ -146,7 +153,7 @@ public class Resolver {
 		}
 
 		// RESOLVE SYNTACTIC CONFLICT BY REMOVING ALL BUT ONE ALT
-		resolveByPickingMinAlt(d, nondeterministicAlts);
+		resolveByPickingMinAlt(d, ambiguousAlts);
 	}
 
 
@@ -166,39 +173,39 @@ public class Resolver {
 	}
 
 	/** Turn off all configurations associated with the
-	 *  set of incoming nondeterministic alts except the min alt number.
+	 *  set of incoming alts except the min alt number.
 	 *  There may be many alts among the configurations but only turn off
 	 *  the ones with problems (other than the min alt of course).
 	 *
-	 *  If nondeterministicAlts is null then turn off all configs 'cept those
+	 *  If alts is null then turn off all configs 'cept those
 	 *  associated with the minimum alt.
 	 *
 	 *  Return the min alt found.
 	 */
-	int resolveByPickingMinAlt(DFAState d, Set<Integer> nondeterministicAlts) {
+	int resolveByPickingMinAlt(DFAState d, Set<Integer> alts) {
 		int min = Integer.MAX_VALUE;
-		if ( nondeterministicAlts!=null ) {
+		if ( alts !=null ) {
-			min = getMinAlt(nondeterministicAlts);
+			min = getMinAlt(alts);
 		}
 		else {
 			min = d.getMinAlt();
 		}
 
-		turnOffOtherAlts(d, min, nondeterministicAlts);
+		turnOffOtherAlts(d, min, alts);
 
 		return min;
 	}
 
 	/** turn off all states associated with alts other than the good one
-	 *  (as long as they are one of the nondeterministic ones)
+	 *  (as long as they are one of the ones in alts)
 	 */
-	void turnOffOtherAlts(DFAState d, int min, Set<Integer> nondeterministicAlts) {
+	void turnOffOtherAlts(DFAState d, int min, Set<Integer> alts) {
 		int numConfigs = d.nfaConfigs.size();
 		for (int i = 0; i < numConfigs; i++) {
 			NFAConfig configuration = d.nfaConfigs.get(i);
 			if ( configuration.alt!=min ) {
-				if ( nondeterministicAlts==null ||
+				if ( alts==null ||
-					 nondeterministicAlts.contains(configuration.alt) )
+					 alts.contains(configuration.alt) )
 				{
 					configuration.resolved = true;
 				}
@@ -206,9 +213,9 @@ public class Resolver {
 		}
 	}
 
-	public static int getMinAlt(Set<Integer> nondeterministicAlts) {
+	public static int getMinAlt(Set<Integer> alts) {
 		int min = Integer.MAX_VALUE;
-		for (Integer altI : nondeterministicAlts) {
+		for (Integer altI : alts) {
 			int alt = altI.intValue();
 			if ( alt < min ) min = alt;
 		}

tool/src/org/antlr/v4/analysis/StackLimitedNFAToDFAConverter.java

@@ -33,25 +33,28 @@ public class StackLimitedNFAToDFAConverter {
      *  lead to this situation (assuming no semantic predicates can resolve
      *  the problem) and when for some reason, I cannot compute the lookahead
      *  (which might arise from an error in the algorithm or from
-     *  left-recursion etc...).  This list starts out with all alts contained
+     *  left-recursion etc...).
-     *  and then in method doesStateReachAcceptState() I remove the alts I
-     *  know to be uniquely predicted.
      */
-    public List<Integer> unreachableAlts;
+    public Set<Integer> unreachableAlts;
 
-	/** Track all DFA states with nondeterministic alternatives.
+	/** Track all DFA states with ambiguous configurations.
 	 *  By reaching the same DFA state, a path through the NFA for some input
 	 *  is able to reach the same NFA state by starting at more than one
-	 *  alternative's left edge.  Though, later, we may find that predicates
+	 *  alternative's left edge. If the context is the same or conflicts,
-	 *  resolve the issue, but track info anyway.
+	 *  then we have ambiguity. If the context is different, it's simply
-	 *  Note that from the DFA state, you can ask for
+	 *  nondeterministic and we should keep looking for edges that will
-	 *  which alts are nondeterministic.
+	 *  render it deterministic. If we run out of things to add to the DFA,
+	 *  we'll get a dangling state; it's non-LL(*). Later we may find that predicates
+	 *  resolve the issue, but track ambiguous states anyway.
 	 */
-	public Set<DFAState> nondeterministicStates = new HashSet<DFAState>();
+	public Set<DFAState> ambiguousStates = new HashSet<DFAState>();
 
 	/** The set of states w/o emanating edges (and w/o resolving sem preds). */
 	public Set<DFAState> danglingStates = new HashSet<DFAState>();
 
+	/** If non-reduced, this is set of states that don't lead to accept state */
+	public Set<DFAState> deadStates;
+
 	/** Was a syntactic ambiguity resolved with predicates?  Any DFA
 	 *  state that predicts more than one alternative, must be resolved
 	 *  with predicates or it should be reported to the user.
@@ -66,6 +69,12 @@ public class StackLimitedNFAToDFAConverter {
 
 	Set<DFAState> recursionOverflowStates = new HashSet<DFAState>();
 
+	/** Are there any loops in this DFA?  Computed by DFAVerifier */
+	public boolean cyclic = false;
+
+	/** Is this DFA reduced?  I.e., can all states lead to an accept state? */
+	public boolean reduced = true;	
+
 	/** Used to prevent the closure operation from looping to itself and
      *  hence looping forever.  Sensitive to the NFA state, the alt, and
      *  the stack context.
@@ -74,6 +83,8 @@ public class StackLimitedNFAToDFAConverter {
 
 	Resolver resolver;
 
+	DFAVerifier verifier;
+	
 	public static boolean debug = false;
 
 	public StackLimitedNFAToDFAConverter(Grammar g, DecisionState nfaStartState) {
@@ -82,10 +93,7 @@ public class StackLimitedNFAToDFAConverter {
 		dfa = new DFA(g, nfaStartState);
 		dfa.converter = this;
 		resolver = new Resolver(this);
-		unreachableAlts = new ArrayList<Integer>();
+		verifier = new DFAVerifier(dfa, this);
-		for (int i = 1; i <= dfa.nAlts; i++) {
-			unreachableAlts.add(i);
-		}		
 	}
 
 	public DFA createDFA() {
@@ -101,6 +109,9 @@ public class StackLimitedNFAToDFAConverter {
 			work.remove(0); // we're done with this DFA state
 		}
 
+		unreachableAlts = verifier.getUnreachableAlts();
+		//deadStates = verifier.getDeadStates();
+
 		return dfa;
 	}
 
@@ -142,7 +153,7 @@ public class StackLimitedNFAToDFAConverter {
 
 	/** Add t if not in DFA yet, resolving nondet's and then make d-label->t */
 	void addTransition(DFAState d, IntervalSet label, DFAState t) {
-		DFAState existing = dfa.uniqueStates.get(t);
+		DFAState existing = dfa.states.get(t);
 		if ( existing != null ) { // seen before; point at old one
 			d.addTransition(new Edge(existing, label));
 			return;
@@ -152,7 +163,7 @@ public class StackLimitedNFAToDFAConverter {
 
 		// resolve any syntactic conflicts by choosing a single alt or
 		// by using semantic predicates if present.
-		resolver.resolveNonDeterminisms(t);
+		resolver.resolveAmbiguities(t);
 
 		// If deterministic, don't add this state; it's an accept state
 		// Just return as a valid DFA state

tool/src/org/antlr/v4/automata/DFA.java

@@ -28,7 +28,7 @@ public class DFA {
 	 *  Not used during fixed k lookahead as it's a waste to fill it with
 	 *  a dup of states array.
      */
-    public Map<DFAState, DFAState> uniqueStates = new HashMap<DFAState, DFAState>();
+    public Map<DFAState, DFAState> states = new HashMap<DFAState, DFAState>();
 
 	/** Maps the state number to the actual DFAState.  This contains all
 	 *  states, but the states are not unique.  s3 might be same as s1 so
@@ -62,12 +62,12 @@ public class DFA {
 
 	/** Add a new DFA state to this DFA (doesn't check if already present). */
 	public void addState(DFAState d) {
-		uniqueStates.put(d,d);
+		states.put(d,d);
 		d.stateNumber = stateCounter++;
 	}
 
 	public void defineAcceptState(int alt, DFAState acceptState) {
-		if ( uniqueStates.get(acceptState)==null ) addState(acceptState);
+		if ( states.get(acceptState)==null ) addState(acceptState);
 		altToAcceptState[alt] = acceptState;
 	}
 	
@@ -80,7 +80,7 @@ public class DFA {
 
 	public boolean isDeterministic() {
 		if ( converter.danglingStates.size()==0 &&
-			 converter.nondeterministicStates.size()==0 &&
+			 converter.ambiguousStates.size()==0 &&
 			 converter.unreachableAlts.size()==0 )
 		{
 			return true;

tool/src/org/antlr/v4/automata/DFAState.java

@@ -50,7 +50,7 @@ public class DFAState {
 	public DFA dfa;
 
 	/** Track the transitions emanating from this DFA state. */
-	protected List<Edge> edges =
+	public List<Edge> edges =
 		new ArrayList<Edge>(INITIAL_NUM_TRANSITIONS);
 
 	/** The set of NFA configurations (state,alt,context) for this DFA state */

tool/src/org/antlr/v4/automata/NFAState.java

@@ -11,6 +11,14 @@ public class NFAState {
 	/** NFAState created for which rule? */
 	public Rule rule;
 
+	/** Which NFA are we in? */
+	public NFA nfa = null;
+
+	/** NFA state is associated with which node in AST? */
+	public GrammarAST ast;
+
+	public NFAState(NFA nfa) { this.nfa = nfa; }
+
 	@Override
 	public int hashCode() {
 		return super.hashCode();
@@ -28,14 +36,6 @@ public class NFAState {
 		return String.valueOf(stateNumber);
 	}
 	
-	/** Which NFA are we in? */
-	public NFA nfa = null;
-
-	/** NFA state is associated with which node in AST? */
-	public GrammarAST ast;
-
-	public NFAState(NFA nfa) { this.nfa = nfa; }
-	
 	public int getNumberOfTransitions() {
 		return 0;
 	}

tool/src/org/antlr/v4/tool/DOTGenerator.java

@@ -20,12 +20,6 @@ public class DOTGenerator {
 	/** Library of output templates; use <attrname> format */
     public static STGroup stlib = new STGroupDir("org/antlr/v4/tool/templates/dot");
 
-    /** To prevent infinite recursion when walking state machines, record
-     *  which states we've visited.  Make a new set every time you start
-     *  walking in case you reuse this object.
-     */
-    protected Set<Integer> markedStates = null;
-
     protected Grammar grammar;
 
     /** This aspect is associated with a grammar */
@@ -38,97 +32,22 @@ public class DOTGenerator {
      *  from startState will be included.
      */
 	public String getDOT(NFAState startState) {
-		if ( startState==null ) {
+		if ( startState==null )	return null;
-			return null;
+
-		}
 		// The output DOT graph for visualization
 		ST dot = null;
-		markedStates = new HashSet<Integer>();
+		Set<NFAState> markedStates = new HashSet<NFAState>();
 		dot = stlib.getInstanceOf("nfa");
-		dot.add("startState",
+		dot.add("startState", Utils.integer(startState.stateNumber));
-				Utils.integer(startState.stateNumber));
-		walkRuleNFACreatingDOT(dot, startState);
 		dot.add("rankdir", rankdir);
-		return dot.render();
-	}
 
-	public String getDOT(DFAState startState) {
+		List<NFAState> work = new LinkedList<NFAState>();
-		if ( startState==null ) {
-			return null;
-		}
-		// The output DOT graph for visualization
-		ST dot = null;
-		markedStates = new HashSet<Integer>();
-		dot = stlib.getInstanceOf("dfa");
-		dot.add("startState", startState.stateNumber);
-		dot.add("useBox", Tool.internalOption_ShowNFAConfigsInDFA);
-		walkCreatingDFADOT(dot,startState);
-		dot.add("rankdir", rankdir);
-		return dot.render();
-	}
 
-    /** Do a depth-first walk of the state machine graph and
+		work.add(startState);
-     *  fill a DOT description template.  Keep filling the
+		while ( work.size()>0 ) {
-     *  states and edges attributes.
+			NFAState s = work.get(0);
-     */
+			if ( markedStates.contains(s) ) { work.remove(0); continue; }
-    protected void walkCreatingDFADOT(ST dot,
+			markedStates.add(s);
-									  DFAState s)
-    {
-		if ( markedStates.contains(Utils.integer(s.stateNumber)) ) {
-			return; // already visited this node
-        }
-
-		markedStates.add(Utils.integer(s.stateNumber)); // mark this node as completed.
-
-        // first add this node
-        ST st;
-        if ( s.isAcceptState ) {
-            st = stlib.getInstanceOf("stopstate");
-        }
-        else {
-            st = stlib.getInstanceOf("state");
-        }
-        st.add("name", getStateLabel(s));
-        dot.add("states", st);
-
-        // make a DOT edge for each transition
-		for (int i = 0; i < s.getNumberOfTransitions(); i++) {
-			Edge edge = s.transition(i);
-			/*
-			System.out.println("dfa "+s.dfa.decisionNumber+
-				" edge from s"+s.stateNumber+" ["+i+"] of "+s.getNumberOfTransitions());
-			*/
-			st = stlib.getInstanceOf("edge");
-//			SemanticContext preds = s.getGatedPredicatesInNFAConfigurations();
-//			if ( preds!=null ) {
-//				String predsStr = "";
-//				predsStr = "&&{"+preds.toString()+"}?";
-//				label += predsStr;
-//			}
-
-			st.add("label", getEdgeLabel(edge.toString(grammar)));
-			st.add("src", getStateLabel(s));
-            st.add("target", getStateLabel(edge.target));
-			st.add("arrowhead", arrowhead);
-            dot.add("edges", st);
-            walkCreatingDFADOT(dot, edge.target); // keep walkin'
-        }
-    }
-
-    /** Do a depth-first walk of the state machine graph and
-     *  fill a DOT description template.  Keep filling the
-     *  states and edges attributes.  We know this is an NFA
-     *  for a rule so don't traverse edges to other rules and
-     *  don't go past rule end state.
-     */
-    protected void walkRuleNFACreatingDOT(ST dot,
-                                          NFAState s)
-    {
-        if ( markedStates.contains(s) ) {
-            return; // already visited this node
-        }
-
-        markedStates.add(s.stateNumber); // mark this node as completed.
 
 			// first add this node
 			ST stateST;
@@ -139,11 +58,9 @@ public class DOTGenerator {
 				stateST = stlib.getInstanceOf("state");
 			}
 			stateST.add("name", getStateLabel(s));
-        dot.add("states", stateST);
 
-        if ( s instanceof RuleStopState )  {
+			// don't go past end of rule node to the follow states
-            return; // don't go past end of rule node to the follow states
+			if ( s instanceof RuleStopState ) continue;
-        }
 
 			// special case: if decision point, then line up the alt start states
 			// unless it's an end of block
@@ -153,7 +70,7 @@ public class DOTGenerator {
 					ST rankST = stlib.getInstanceOf("decision-rank");
 					NFAState alt = (NFAState)s;
 					while ( alt!=null ) {
-					rankST.add("states", getStateLabel(alt));
+						rankST.add("states", alt.stateNumber);
 						if ( alt.transition(1) !=null ) {
 							alt = (NFAState)alt.transition(1).target;
 						}
@@ -179,11 +96,11 @@ public class DOTGenerator {
 					else {
 						edgeST.add("label", "<"+rr.rule.name+">");
 					}
-				edgeST.add("src", getStateLabel(s));
+					edgeST.add("src", s.stateNumber);
-				edgeST.add("target", getStateLabel(rr.followState));
+					edgeST.add("target", rr.followState.stateNumber);
 					edgeST.add("arrowhead", arrowhead);
 					dot.add("edges", edgeST);
-				walkRuleNFACreatingDOT(dot, rr.followState);
+					work.add(rr.followState);
 					continue;
 				}
 				if ( edge instanceof ActionTransition ) {
@@ -199,14 +116,183 @@ public class DOTGenerator {
 					edgeST = stlib.getInstanceOf("edge");
 				}
 				edgeST.add("label", getEdgeLabel(edge.toString(grammar)));
-            edgeST.add("src", getStateLabel(s));
+				edgeST.add("src", s.stateNumber);
-			edgeST.add("target", getStateLabel(edge.target));
+				edgeST.add("target", edge.target.stateNumber);
 				edgeST.add("arrowhead", arrowhead);
 				dot.add("edges", edgeST);
-            walkRuleNFACreatingDOT(dot, edge.target); // keep walkin'
+				work.add(edge.target);
 			}
 		}
 
+		// define nodes we visited (they will appear first in DOT output)
+		// this is an example of ST's lazy eval :)
+		for (NFAState s : markedStates) {
+			ST st;
+			if ( s instanceof RuleStopState ) {
+				st = stlib.getInstanceOf("stopstate");
+			}
+			else {
+				st = stlib.getInstanceOf("state");
+			}
+			st.add("name", s.stateNumber);
+			st.add("label", getStateLabel(s));
+			dot.add("states", st);
+		}
+
+		return dot.render();
+	}
+
+	public String getDOT(DFAState startState) {
+		if ( startState==null )	return null;
+
+		ST dot = stlib.getInstanceOf("dfa");
+		dot.add("startState", startState.stateNumber);
+		dot.add("useBox", Tool.internalOption_ShowNFAConfigsInDFA);
+		dot.add("rankdir", rankdir);
+
+		Set<DFAState> markedStates = new HashSet<DFAState>();
+		List<DFAState> work = new LinkedList<DFAState>();
+
+		for (DFAState d : startState.dfa.states.values()) {
+			ST st;
+			if ( d.isAcceptState ) {
+				st = stlib.getInstanceOf("stopstate");
+			}
+			else {
+				st = stlib.getInstanceOf("state");
+			}
+			st.add("name", "s"+d.stateNumber);
+			st.add("label", getStateLabel(d));
+			dot.add("states", st);
+		}
+
+		work.add(startState);
+		while ( work.size()>0 ) {
+			DFAState d = work.get(0);
+			if ( markedStates.contains(d) ) { work.remove(0); continue; }
+			markedStates.add(d);
+			
+			// make a DOT edge for each transition
+			for (int i = 0; i < d.getNumberOfTransitions(); i++) {
+				Edge edge = d.transition(i);
+				/*
+				System.out.println("dfa "+s.dfa.decisionNumber+
+					" edge from s"+s.stateNumber+" ["+i+"] of "+s.getNumberOfTransitions());
+				*/
+				ST st = stlib.getInstanceOf("edge");
+//			SemanticContext preds = s.getGatedPredicatesInNFAConfigurations();
+//			if ( preds!=null ) {
+//				String predsStr = "";
+//				predsStr = "&&{"+preds.toString()+"}?";
+//				label += predsStr;
+//			}
+
+				st.add("label", getEdgeLabel(edge.toString(grammar)));
+				st.add("src", "s"+d.stateNumber);
+				st.add("target", "s"+edge.target.stateNumber);
+				st.add("arrowhead", arrowhead);
+				dot.add("edges", st);
+				work.add(edge.target); // add targets to list of states to visit
+			}
+
+			work.remove(0);
+		}
+
+		return dot.render();
+	}
+
+    /** Do a depth-first walk of the state machine graph and
+     *  fill a DOT description template.  Keep filling the
+     *  states and edges attributes.  We know this is an NFA
+     *  for a rule so don't traverse edges to other rules and
+     *  don't go past rule end state.
+     */
+//    protected void walkRuleNFACreatingDOT(ST dot,
+//                                          NFAState s)
+//    {
+//        if ( markedStates.contains(s) ) {
+//            return; // already visited this node
+//        }
+//
+//        markedStates.add(s.stateNumber); // mark this node as completed.
+//
+//        // first add this node
+//        ST stateST;
+//        if ( s instanceof RuleStopState ) {
+//            stateST = stlib.getInstanceOf("stopstate");
+//        }
+//        else {
+//            stateST = stlib.getInstanceOf("state");
+//        }
+//        stateST.add("name", getStateLabel(s));
+//        dot.add("states", stateST);
+//
+//        if ( s instanceof RuleStopState )  {
+//            return; // don't go past end of rule node to the follow states
+//        }
+//
+//        // special case: if decision point, then line up the alt start states
+//        // unless it's an end of block
+//		if ( s instanceof DecisionState ) {
+//			GrammarAST n = ((NFAState)s).ast;
+//			if ( n!=null && s instanceof BlockEndState ) {
+//				ST rankST = stlib.getInstanceOf("decision-rank");
+//				NFAState alt = (NFAState)s;
+//				while ( alt!=null ) {
+//					rankST.add("states", getStateLabel(alt));
+//					if ( alt.transition(1) !=null ) {
+//						alt = (NFAState)alt.transition(1).target;
+//					}
+//					else {
+//						alt=null;
+//					}
+//				}
+//				dot.add("decisionRanks", rankST);
+//			}
+//		}
+//
+//        // make a DOT edge for each transition
+//		ST edgeST = null;
+//		for (int i = 0; i < s.getNumberOfTransitions(); i++) {
+//            Transition edge = (Transition) s.transition(i);
+//            if ( edge instanceof RuleTransition ) {
+//                RuleTransition rr = ((RuleTransition)edge);
+//                // don't jump to other rules, but display edge to follow node
+//                edgeST = stlib.getInstanceOf("edge");
+//				if ( rr.rule.g != grammar ) {
+//					edgeST.add("label", "<"+rr.rule.g.name+"."+rr.rule.name+">");
+//				}
+//				else {
+//					edgeST.add("label", "<"+rr.rule.name+">");
+//				}
+//				edgeST.add("src", getStateLabel(s));
+//				edgeST.add("target", getStateLabel(rr.followState));
+//				edgeST.add("arrowhead", arrowhead);
+//                dot.add("edges", edgeST);
+//				walkRuleNFACreatingDOT(dot, rr.followState);
+//                continue;
+//            }
+//			if ( edge instanceof ActionTransition ) {
+//				edgeST = stlib.getInstanceOf("action-edge");
+//			}
+//			else if ( edge instanceof PredicateTransition ) {
+//				edgeST = stlib.getInstanceOf("edge");
+//			}
+//			else if ( edge.isEpsilon() ) {
+//				edgeST = stlib.getInstanceOf("epsilon-edge");
+//			}
+//			else {
+//				edgeST = stlib.getInstanceOf("edge");
+//			}
+//			edgeST.add("label", getEdgeLabel(edge.toString(grammar)));
+//            edgeST.add("src", getStateLabel(s));
+//			edgeST.add("target", getStateLabel(edge.target));
+//			edgeST.add("arrowhead", arrowhead);
+//            dot.add("edges", edgeST);
+//            walkRuleNFACreatingDOT(dot, edge.target); // keep walkin'
+//        }
+//    }
+
     /** Fix edge strings so they print out in DOT properly;
 	 *  generate any gated predicates on edge too.
 	 */
@@ -222,14 +308,13 @@ public class DOTGenerator {
 		if ( s==null ) return "null";
 		String stateLabel = String.valueOf(s.stateNumber);
 		if ( s instanceof DecisionState ) {
-			stateLabel = stateLabel+",d="+((DecisionState)s).decision;
+			stateLabel = stateLabel+"\\nd="+((DecisionState)s).decision;
 		}
-		return '"'+stateLabel+'"';
+		return stateLabel;
 	}
 
 	protected String getStateLabel(DFAState s) {
 		if ( s==null ) return "null";
-		String stateLabel = String.valueOf(s.stateNumber);
 		StringBuffer buf = new StringBuffer(250);
 		buf.append('s');
 		buf.append(s.stateNumber);
@@ -275,10 +360,10 @@ public class DOTGenerator {
 				}
 			}
 		}
-		stateLabel = buf.toString();
+		String stateLabel = buf.toString();
 		if ( s.isAcceptState ) {
             stateLabel = stateLabel+"=>"+s.getUniquelyPredictedAlt();
         }
-        return '"'+stateLabel+'"';
+        return stateLabel;
     }
 }