Change the way hard splits are handled.

By having a separate rule for hard splits, we lose any constraints the
original soft rule may have had or -- more importantly -- other rules
may have had on it.

For constraints where hardening the rule forces other rules to fully
harden, it's fine because we propagate those constraints when the rule
is hardened. The problem comes with "cannot split" constraints. If rule
A has a "cannot split" constraint on rule B when rule A has some value,
we need to remember that even if B is hardened so that we prevent A
from taking that value.

Instead of swapping out the rule with a separate hard split rule, the
idea is to keep the rule (and thus preserve references to it) but change
it to a "hardened" state.

R=kevmoo@google.com

Review URL: https://codereview.chromium.org//1492683002 .

lib/src/argument_list_visitor.dart

@@ -290,7 +290,7 @@ class ArgumentSublist {
 
   void visit(SourceVisitor visitor) {
     if (_collections.isNotEmpty) {
-      _collectionRule = new SimpleRule(Cost.splitCollections);
+      _collectionRule = new Rule(Cost.splitCollections);
     }
 
     var rule = _visitPositional(visitor);

lib/src/chunk.dart

@@ -114,10 +114,6 @@ class Chunk extends Selection {
   Rule get rule => _rule;
   Rule _rule;
 
-  /// Whether this chunk is always followed by a newline or whether the line
-  /// splitter may choose to keep the next chunk on the same line.
-  bool get isHardSplit => _rule is HardSplitRule;
-
   /// Whether or not an extra blank line should be output after this chunk if
   /// it's split.
   ///
@@ -197,15 +193,6 @@ class Chunk extends Selection {
     _text += text;
   }
 
-  /// Forces this soft split to become a hard split.
-  ///
-  /// This is called on the soft splits owned by a rule that decides to harden
-  /// when it finds out another hard split occurs within its chunks.
-  void harden() {
-    _rule = new HardSplitRule();
-    spans.clear();
-  }
-
   /// Finishes off this chunk with the given [rule] and split information.
   ///
   /// This may be called multiple times on the same split since the splits
@@ -216,7 +203,7 @@ class Chunk extends Selection {
       {bool flushLeft, bool isDouble, bool space}) {
     if (flushLeft == null) flushLeft = false;
     if (space == null) space = false;
-    if (isHardSplit || rule is HardSplitRule) {
+    if (rule.isHardened) {
       // A hard split always wins.
       _rule = rule;
     } else if (_rule == null) {
@@ -271,10 +258,9 @@ class Chunk extends Selection {
 
     if (_rule == null) {
       parts.add("(no split)");
-    } else if (isHardSplit) {
-      parts.add("hard");
     } else {
       parts.add(rule.toString());
+      if (rule.isHardened) parts.add("(hard)");
 
       if (_rule.constrainedRules.isNotEmpty) {
         parts.add("-> ${_rule.constrainedRules.join(' ')}");

lib/src/chunk_builder.dart

@@ -361,15 +361,15 @@ class ChunkBuilder {
     var span = new Span(openSpan.cost);
     for (var i = openSpan.start; i < end; i++) {
       var chunk = _chunks[i];
-      if (!chunk.isHardSplit) chunk.spans.add(span);
+      if (!chunk.rule.isHardened) chunk.spans.add(span);
     }
   }
 
   /// Starts a new [Rule].
   ///
-  /// If omitted, defaults to a new [SimpleRule].
+  /// If omitted, defaults to a new [Rule].
   void startRule([Rule rule]) {
-    if (rule == null) rule = new SimpleRule();
+    if (rule == null) rule = new Rule();
 
     // See if any of the rules that contain this one care if it splits.
     _rules.forEach((outer) => outer.contain(rule));
@@ -383,9 +383,9 @@ class ChunkBuilder {
   /// first operand but not get forced to split if a comment appears before the
   /// entire expression.
   ///
-  /// If [rule] is omitted, defaults to a new [SimpleRule].
+  /// If [rule] is omitted, defaults to a new [Rule].
   void startLazyRule([Rule rule]) {
-    if (rule == null) rule = new SimpleRule();
+    if (rule == null) rule = new Rule();
 
     _lazyRules.add(rule);
   }
@@ -490,7 +490,7 @@ class ChunkBuilder {
   /// `true`, the block is considered to always split.
   ///
   /// Returns the previous writer for the surrounding block.
-  ChunkBuilder endBlock(HardSplitRule ignoredSplit, {bool forceSplit}) {
+  ChunkBuilder endBlock(Rule ignoredSplit, {bool forceSplit}) {
     _divideChunks();
 
     // If we don't already know if the block is going to split, see if it
@@ -504,7 +504,9 @@ class ChunkBuilder {
           break;
         }
 
-        if (chunk.isHardSplit && chunk.rule != ignoredSplit) {
+        if (chunk.rule != null &&
+            chunk.rule.isHardened &&
+            chunk.rule != ignoredSplit) {
           forceSplit = true;
           break;
         }
@@ -682,7 +684,7 @@ class ChunkBuilder {
   void _writeHardSplit({bool isDouble, bool flushLeft, bool nest: false}) {
     // A hard split overrides any other whitespace.
     _pendingWhitespace = null;
-    _writeSplit(new HardSplitRule(),
+    _writeSplit(new Rule.hard(),
         flushLeft: flushLeft, isDouble: isDouble, nest: nest);
   }
 
@@ -711,11 +713,11 @@ class ChunkBuilder {
   Chunk _afterSplit() {
     var chunk = _chunks.last;
 
-    if (chunk.rule is! HardSplitRule) {
+    if (_rules.isNotEmpty) {
       _ruleChunks.putIfAbsent(rule, () => []).add(_chunks.length - 1);
     }
 
-    if (chunk.isHardSplit) _handleHardSplit();
+    if (chunk.rule.isHardened) _handleHardSplit();
 
     return chunk;
   }
@@ -733,8 +735,11 @@ class ChunkBuilder {
   /// Returns true if we can divide the chunks at [index] and line split the
   /// ones before and after that separately.
   bool _canDivideAt(int i) {
+    // Don't divide after the last chunk.
+    if (i == _chunks.length - 1) return false;
+
     var chunk = _chunks[i];
-    if (!chunk.isHardSplit) return false;
+    if (!chunk.rule.isHardened) return false;
     if (chunk.nesting.isNested) return false;
     if (chunk.isBlock) return false;
 
@@ -782,18 +787,16 @@ class ChunkBuilder {
   void _hardenRules() {
     if (_hardSplitRules.isEmpty) return;
 
-    // Harden all of the rules that are constrained by [rules] as well.
-    var hardenedRules = new Set();
     walkConstraints(rule) {
-      if (hardenedRules.contains(rule)) return;
-      hardenedRules.add(rule);
+      rule.harden();
 
       // Follow this rule's constraints, recursively.
       for (var other in _ruleChunks.keys) {
         if (other == rule) continue;
 
-        if (rule.constrain(rule.fullySplitValue, other) ==
-            other.fullySplitValue) {
+        if (!other.isHardened &&
+            rule.constrain(rule.fullySplitValue, other) ==
+                other.fullySplitValue) {
           walkConstraints(other);
         }
       }
@@ -803,10 +806,11 @@ class ChunkBuilder {
       walkConstraints(rule);
     }
 
-    // Harden every chunk that uses one of these rules.
+    // Discard spans in hardened chunks since we know for certain they will
+    // split anyway.
     for (var chunk in _chunks) {
-      if (hardenedRules.contains(chunk.rule)) {
-        chunk.harden();
+      if (chunk.rule != null && chunk.rule.isHardened) {
+        chunk.spans.clear();
       }
     }
   }

lib/src/debug.dart

@@ -84,10 +84,8 @@ void dumpChunks(int start, List<Chunk> chunks) {
 
   spans = spans.toList();
 
-  var rules = chunks
-      .map((chunk) => chunk.rule)
-      .where((rule) => rule != null && rule is! HardSplitRule)
-      .toSet();
+  var rules =
+      chunks.map((chunk) => chunk.rule).where((rule) => rule != null).toSet();
 
   var rows = [];
 
@@ -132,13 +130,12 @@ void dumpChunks(int start, List<Chunk> chunks) {
       row.add("");
       row.add("(no rule)");
       row.add("");
-    } else if (chunk.isHardSplit) {
-      row.add("");
-      row.add("(hard)");
-      row.add("");
-    } else if (chunk.rule != null) {
+    } else {
       writeIf(chunk.rule.cost != 0, () => "\$${chunk.rule.cost}");
-      row.add(chunk.rule.toString());
+
+      var ruleString = chunk.rule.toString();
+      if (chunk.rule.isHardened) ruleString += "!";
+      row.add(ruleString);
 
       var constrainedRules =
           chunk.rule.constrainedRules.toSet().intersection(rules);
@@ -191,6 +188,33 @@ void dumpChunks(int start, List<Chunk> chunks) {
   print(buffer.toString());
 }
 
+/// Shows all of the constraints between the rules used by [chunks].
+void dumpConstraints(List<Chunk> chunks) {
+  var rules =
+      chunks.map((chunk) => chunk.rule).where((rule) => rule != null).toSet();
+
+  for (var rule in rules) {
+    var constrainedValues = [];
+    for (var value = 0; value < rule.numValues; value++) {
+      var constraints = [];
+      for (var other in rules) {
+        if (rule == other) continue;
+
+        var constraint = rule.constrain(value, other);
+        if (constraint != null) {
+          constraints.add("$other->$constraint");
+        }
+      }
+
+      if (constraints.isNotEmpty) {
+        constrainedValues.add("$value:(${constraints.join(' ')})");
+      }
+    }
+
+    log("$rule ${constrainedValues.join(' ')}");
+  }
+}
+
 /// Convert the line to a [String] representation.
 ///
 /// It will determine how best to split it into multiple lines of output and

lib/src/line_splitting/line_splitter.dart

@@ -130,10 +130,10 @@ class LineSplitter {
       int firstLineIndent,
       {bool flushLeft: false})
       : chunks = chunks,
-        // Collect the set of soft rules that we need to select values for.
+        // Collect the set of rules that we need to select values for.
         rules = chunks
             .map((chunk) => chunk.rule)
-            .where((rule) => rule != null && rule is! HardSplitRule)
+            .where((rule) => rule != null)
             .toSet()
             .toList(growable: false),
         blockIndentation = blockIndentation,

lib/src/line_splitting/rule_set.dart

@@ -23,10 +23,18 @@ class RuleSet {
   RuleSet._(this._values);
 
   /// Returns `true` of [rule] is bound in this set.
-  bool contains(Rule rule) => _values[rule.index] != null;
+  bool contains(Rule rule) {
+    // Treat hardened rules as implicitly bound.
+    if (rule.isHardened) return true;
+
+    return _values[rule.index] != null;
+  }
 
   /// Gets the bound value for [rule] or [Rule.unsplit] if it is not bound.
   int getValue(Rule rule) {
+    // Hardened rules are implicitly bound.
+    if (rule.isHardened) return rule.fullySplitValue;
+
     var value = _values[rule.index];
     if (value != null) return value;
 
@@ -56,11 +64,20 @@ class RuleSet {
   /// If an unbound rule gets constrained to `-1` (meaning it must split, but
   /// can split any way it wants), invokes [onSplitRule] with it.
   bool tryBind(List<Rule> rules, Rule rule, int value, onSplitRule(Rule rule)) {
+    assert(!rule.isHardened);
+
     _values[rule.index] = value;
 
     // Test this rule against the other rules being bound.
     for (var other in rule.constrainedRules) {
-      var otherValue = _values[other.index];
+      var otherValue;
+      // Hardened rules are implicitly bound.
+      if (other.isHardened) {
+        otherValue = other.fullySplitValue;
+      } else {
+        otherValue = _values[other.index];
+      }
+
       var constraint = rule.constrain(value, other);
 
       if (otherValue == null) {

lib/src/line_splitting/solve_state.dart

@@ -113,11 +113,7 @@ class SolveState {
 
   /// Gets the value to use for [rule], either the bound value or
   /// [Rule.unsplit] if it isn't bound.
-  int getValue(Rule rule) {
-    if (rule is HardSplitRule) return Rule.unsplit;
-
-    return _ruleValues.getValue(rule);
-  }
+  int getValue(Rule rule) => _ruleValues.getValue(rule);
 
   /// Returns `true` if this state is a better solution to use as the final
   /// result than [other].
@@ -437,7 +433,6 @@ class SolveState {
   /// live rules were added.
   bool _addLiveRules(Rule rule) {
     if (rule == null) return false;
-    if (rule is HardSplitRule) return false;
 
     var added = false;
     for (var constrained in rule.allConstrainedRules) {
@@ -472,7 +467,7 @@ class SolveState {
       }
 
       var rule = _splitter.chunks[i].rule;
-      if (rule != null && rule is! HardSplitRule) {
+      if (rule != null) {
         if (_ruleValues.contains(rule)) {
           boundInLine.add(rule);
         } else {

lib/src/line_writer.dart

@@ -144,7 +144,7 @@ class LineWriter {
 
     if (debug.traceLineWriter) {
       debug.log(debug.green("\nWriting:"));
-      debug.dumpChunks(start, chunks);
+      debug.dumpChunks(0, chunks);
       debug.log();
     }
 

lib/src/rule/argument.dart

@@ -124,8 +124,6 @@ class SinglePositionalRule extends PositionalRule {
         splitsOnInnerRules =
             splitsOnInnerRules != null ? splitsOnInnerRules : false;
 
-  bool isSplit(int value, Chunk chunk) => value != Rule.unsplit;
-
   int constrain(int value, Rule other) {
     var constrained = super.constrain(value, other);
     if (constrained != null) return constrained;
@@ -194,10 +192,7 @@ class MultiplePositionalRule extends PositionalRule {
 
   String toString() => "*Pos${super.toString()}";
 
-  bool isSplit(int value, Chunk chunk) {
-    // Don't split at all.
-    if (value == Rule.unsplit) return false;
-
+  bool isSplitAtValue(int value, Chunk chunk) {
     // Split only before the first argument. Keep the entire argument list
     // together on the next line.
     if (value == 1) return chunk == _arguments.first;
@@ -298,17 +293,12 @@ class NamedRule extends ArgumentRule {
     _first = chunk;
   }
 
-  bool isSplit(int value, Chunk chunk) {
-    switch (value) {
-      case Rule.unsplit:
-        return false;
-      case 1:
-        return chunk == _first;
-      case 2:
-        return true;
-    }
+  bool isSplitAtValue(int value, Chunk chunk) {
+    // Only split before the first argument.
+    if (value == 1) return chunk == _first;
 
-    throw "unreachable";
+    // Split before every argument.
+    return true;
   }
 
   String toString() => "Named${super.toString()}";