Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2)

Unified Diff: packages/dart_style/lib/src/line_splitting/solve_state.dart

Issue 1521693002: Roll Observatory deps (charted -> ^0.3.0) (Closed) Base URL: https://chromium.googlesource.com/external/github.com/dart-lang/observatory_pub_packages.git@master
Patch Set: Created 5 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: packages/dart_style/lib/src/line_splitting/solve_state.dart
diff --git a/packages/dart_style/lib/src/line_splitting/solve_state.dart b/packages/dart_style/lib/src/line_splitting/solve_state.dart
index b4197561aa9fc9912e3018ef00391b0d09e9c78c..e1e9168c2a2570e90e5c7b149978e27c1776ab0f 100644
--- a/packages/dart_style/lib/src/line_splitting/solve_state.dart
+++ b/packages/dart_style/lib/src/line_splitting/solve_state.dart
@@ -6,6 +6,7 @@ library dart_style.src.line_splitting.solve_state;
import '../debug.dart' as debug;
import '../rule/rule.dart';
+import '../whitespace.dart';
import 'line_splitter.dart';
import 'rule_set.dart';
@@ -25,11 +26,21 @@ class SolveState {
final LineSplitter _splitter;
final RuleSet _ruleValues;
+ /// The set of [Rule]s that are bound by [_ruleValues].
+ ///
+ /// Cached by [_ensureConstraints] for use by [_ensureUnboundConstraints].
+ Set<Rule> _boundRules;
+
+ /// The set of [Rule]s that are not bound by [_ruleValues].
+ ///
+ /// Cached by [_ensureConstraints] for use by [_ensureUnboundConstraints].
+ Set<Rule> _unboundRules;
+
/// The unbound rules in this state that can be bound to produce new more
/// refined states.
///
/// Keeping this set small is the key to make the entire line splitter
- /// perform well. If we consider too make rules at each state, our
+ /// perform well. If we consider too many rules at each state, our
/// exploration of the solution space is too branchy and we waste time on
/// dead end solutions.
///
@@ -75,6 +86,19 @@ class SolveState {
/// unbound rules.
Map<Rule, int> _constraints;
+ /// The unbound rule values that are disallowed because they would place
+ /// invalid constraints on the currently bound values.
+ ///
+ /// For example, say rule A is bound to 1 and B is unbound. If B has value
+ /// 1, it constrains A to be 1. Likewise, if B is 2, it constrains A to be
+ /// 2 as well. Since we already know A is 1, that means we know B cannot be
+ /// bound to value 2. This means B is more limited in this state than it
+ /// might be in another state that binds A to a different value.
+ ///
+ /// It's important to track this, because we can't allow to states to overlap
+ /// if one permits more values for some unbound rule than the other does.
+ Map<Rule, Set<int>> _unboundConstraints;
+
/// The bound rules that appear inside lines also containing unbound rules.
///
/// By appearing in the same line, it means these bound rules may affect the
@@ -87,13 +111,9 @@ class SolveState {
_calculateCost();
}
- /// Gets the value to use for [rule], either the bound value or `0` if it
- /// isn't bound.
- int getValue(Rule rule) {
- if (rule is HardSplitRule) return 0;
-
- return _ruleValues.getValue(rule);
- }
+ /// Gets the value to use for [rule], either the bound value or
+ /// [Rule.unsplit] if it isn't bound.
+ int getValue(Rule rule) => _ruleValues.getValue(rule);
/// Returns `true` if this state is a better solution to use as the final
/// result than [other].
@@ -212,11 +232,10 @@ class SolveState {
/// Returns `true` if [other] overlaps this state.
bool _isOverlapping(SolveState other) {
- _ensureOverlapFields();
- other._ensureOverlapFields();
-
// Lines that contain both bound and unbound rules must have the same
// bound values.
+ _ensureBoundRulesInUnboundLines();
+ other._ensureBoundRulesInUnboundLines();
if (_boundRulesInUnboundLines.length !=
other._boundRulesInUnboundLines.length) {
return false;
@@ -229,12 +248,30 @@ class SolveState {
}
}
+ _ensureConstraints();
+ other._ensureConstraints();
if (_constraints.length != other._constraints.length) return false;
for (var rule in _constraints.keys) {
if (_constraints[rule] != other._constraints[rule]) return false;
}
+ _ensureUnboundConstraints();
+ other._ensureUnboundConstraints();
+ if (_unboundConstraints.length != other._unboundConstraints.length) {
+ return false;
+ }
+
+ for (var rule in _unboundConstraints.keys) {
+ var disallowed = _unboundConstraints[rule];
+ var otherDisallowed = other._unboundConstraints[rule];
+
+ if (disallowed.length != otherDisallowed.length) return false;
+ for (var value in disallowed) {
+ if (!otherDisallowed.contains(value)) return false;
+ }
+ }
+
return true;
}
@@ -267,6 +304,8 @@ class SolveState {
// And any expression nesting.
indent += chunk.nesting.totalUsedIndent;
+
+ if (chunk.indentBlock(getValue)) indent += Indent.expression;
}
_splits.add(i, indent);
@@ -288,7 +327,8 @@ class SolveState {
// The unbound rules in use by the current line. This will be null after
// the first long line has completed.
- var currentLineRules = [];
+ var foundOverflowRules = false;
+ var start = 0;
endLine(int end) {
// Track lines that went over the length. It is only rules contained in
@@ -298,16 +338,16 @@ class SolveState {
// Only try rules that are in the first long line, since we know at
// least one of them *will* be split.
- if (currentLineRules != null && currentLineRules.isNotEmpty) {
- _liveRules.addAll(currentLineRules);
- currentLineRules = null;
- }
- } else {
- // The line fit, so don't keep track of its rules.
- if (currentLineRules != null) {
- currentLineRules.clear();
+ if (!foundOverflowRules) {
+ for (var i = start; i < end; i++) {
+ if (_addLiveRules(_splitter.chunks[i].rule)) {
+ foundOverflowRules = true;
+ }
+ }
}
}
+
+ start = end;
}
// The set of spans that contain chunks that ended up splitting. We store
@@ -315,6 +355,9 @@ class SolveState {
// one split occurs in it.
var splitSpans = new Set();
+ // The nesting level of the chunk that ended the previous line.
+ var previousNesting;
+
for (var i = 0; i < _splitter.chunks.length; i++) {
var chunk = _splitter.chunks[i];
@@ -329,11 +372,37 @@ class SolveState {
splitSpans.addAll(chunk.spans);
// Include the cost of the nested block.
- if (chunk.blockChunks.isNotEmpty) {
+ if (chunk.isBlock) {
cost +=
_splitter.writer.formatBlock(chunk, _splits.getColumn(i)).cost;
}
+ // Do not allow sequential lines to have the same indentation but for
+ // different reasons. In other words, don't allow different expressions
+ // to claim the same nesting level on subsequent lines.
+ //
+ // A contrived example would be:
+ //
+ // function(inner(
+ // argument), second(
+ // another);
+ //
+ // For the most part, we prevent this by the constraints on splits.
+ // For example, the above can't happen because the split before
+ // "argument", forces the split before "second".
+ //
+ // But there are a couple of squirrely cases where it's hard to prevent
+ // by construction. Instead, this outlaws it by penalizing it very
+ // heavily if it happens to get this far.
+ if (previousNesting != null &&
+ chunk.nesting.totalUsedIndent != 0 &&
+ chunk.nesting.totalUsedIndent == previousNesting.totalUsedIndent &&
+ !identical(chunk.nesting, previousNesting)) {
+ _overflowChars += 10000;
+ }
+
+ previousNesting = chunk.nesting;
+
// Start the new line.
length = _splits.getColumn(i);
} else {
@@ -341,23 +410,12 @@ class SolveState {
// Include the nested block inline, if any.
length += chunk.unsplitBlockLength;
-
- // If we might be in the first overly long line, keep track of any
- // unbound rules we encounter. These are ones that we'll want to try to
- // bind to shorten the long line.
- if (currentLineRules != null &&
- chunk.rule != null &&
- !chunk.isHardSplit &&
- !_ruleValues.contains(chunk.rule)) {
- currentLineRules.add(chunk.rule);
- }
}
}
- // Add the costs for the rules that split.
+ // Add the costs for the rules that have any splits.
_ruleValues.forEach(_splitter.rules, (rule, value) {
- // A rule may be bound to zero if another rule constrains it to not split.
- if (value != 0) cost += rule.cost;
+ if (value != Rule.unsplit) cost += rule.cost;
});
// Add the costs for the spans containing splits.
@@ -369,46 +427,32 @@ class SolveState {
_splits.setCost(cost);
}
- /// Lazily initializes the fields needed to compare two states for overlap.
+ /// Adds [rule] and all of the rules it constrains to the set of [_liveRules].
///
- /// We do this lazily because the calculation is a bit slow, and is only
- /// needed when we have two states with the same score.
- void _ensureOverlapFields() {
- if (_constraints != null) return;
-
- _calculateConstraints();
- _calculateBoundRulesInUnboundLines();
- }
+ /// Only does this if [rule] is a valid soft rule. Returns `true` if any new
+ /// live rules were added.
+ bool _addLiveRules(Rule rule) {
+ if (rule == null) return false;
- /// Initializes [_constraints] with any constraints the bound rules place on
- /// the unbound rules.
- void _calculateConstraints() {
- _constraints = {};
-
- var unboundRules = [];
- var boundRules = [];
+ var added = false;
+ for (var constrained in rule.allConstrainedRules) {
+ if (_ruleValues.contains(constrained)) continue;
- for (var rule in _splitter.rules) {
- if (_ruleValues.contains(rule)) {
- boundRules.add(rule);
- } else {
- unboundRules.add(rule);
- }
+ _liveRules.add(constrained);
+ added = true;
}
- for (var bound in boundRules) {
- var value = _ruleValues.getValue(bound);
-
- for (var unbound in unboundRules) {
- var constraint = bound.constrain(value, unbound);
- if (constraint != null) {
- _constraints[unbound] = constraint;
- }
- }
- }
+ return added;
}
- void _calculateBoundRulesInUnboundLines() {
+ /// Lazily initializes the [_boundInUnboundLines], which is needed to compare
+ /// two states for overlap.
+ ///
+ /// We do this lazily because the calculation is a bit slow, and is only
+ /// needed when we have two states with the same score.
+ void _ensureBoundRulesInUnboundLines() {
+ if (_boundRulesInUnboundLines != null) return;
+
_boundRulesInUnboundLines = new Set();
var boundInLine = new Set();
@@ -423,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 {
@@ -435,6 +479,89 @@ class SolveState {
if (hasUnbound) _boundRulesInUnboundLines.addAll(boundInLine);
}
+ /// Lazily initializes the [_constraints], which is needed to compare two
+ /// states for overlap.
+ ///
+ /// We do this lazily because the calculation is a bit slow, and is only
+ /// needed when we have two states with the same score.
+ void _ensureConstraints() {
+ if (_constraints != null) return;
+
+ _unboundRules = new Set();
+ _boundRules = new Set();
+
+ for (var rule in _splitter.rules) {
+ if (_ruleValues.contains(rule)) {
+ _boundRules.add(rule);
+ } else {
+ _unboundRules.add(rule);
+ }
+ }
+
+ _constraints = {};
+
+ for (var bound in _boundRules) {
+ for (var unbound in bound.constrainedRules) {
+ if (!_unboundRules.contains(unbound)) continue;
+
+ var value = _ruleValues.getValue(bound);
+ var constraint = bound.constrain(value, unbound);
+ if (constraint != null) {
+ _constraints[unbound] = constraint;
+ }
+ }
+ }
+ }
+
+ /// Lazily initializes the [_unboundConstraints], which is needed to compare
+ /// two states for overlap.
+ ///
+ /// We do this lazily because the calculation is a bit slow, and is only
+ /// needed when we have two states with the same score.
+ void _ensureUnboundConstraints() {
+ if (_unboundConstraints != null) return;
+
+ // _ensureConstraints should be called first which initializes these.
+ assert(_boundRules != null);
+ assert(_unboundRules != null);
+
+ _unboundConstraints = {};
+
+ for (var unbound in _unboundRules) {
+ var disallowedValues;
+
+ for (var bound in unbound.constrainedRules) {
+ if (!_boundRules.contains(bound)) continue;
+
+ var boundValue = _ruleValues.getValue(bound);
+
+ for (var value = 0; value < unbound.numValues; value++) {
+ var constraint = unbound.constrain(value, bound);
+
+ // If the unbound rule doesn't place any constraint on this bound
+ // rule, we're fine.
+ if (constraint == null) continue;
+
+ // If the bound rule's value already meets the constraint it applies,
+ // we don't need to track it. This way, two states that have the
+ // same bound value, one of which has a satisfied constraint, are
+ // still allowed to overlap.
+ if (constraint == boundValue) continue;
+ if (constraint == Rule.mustSplit && boundValue != Rule.unsplit) {
+ continue;
+ }
+
+ if (disallowedValues == null) {
+ disallowedValues = new Set();
+ _unboundConstraints[unbound] = disallowedValues;
+ }
+
+ disallowedValues.add(value);
+ }
+ }
+ }
+ }
+
String toString() {
var buffer = new StringBuffer();
« no previous file with comments | « packages/dart_style/lib/src/line_splitting/rule_set.dart ('k') | packages/dart_style/lib/src/line_writer.dart » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698