Skip to content

Fix bad offset, underflow issues #232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 31, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 34 additions & 25 deletions src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,8 @@ fn rewrite_closure(capture: ast::CaptureClause,

// 1 = the separating space between arguments and the body.
let extra_offset = extra_offset(&prefix, offset) + 1;
let rewrite = inner_expr.rewrite(context, width - extra_offset, offset + extra_offset);
let budget = try_opt!(width.checked_sub(extra_offset));
let rewrite = inner_expr.rewrite(context, budget, offset + extra_offset);

// Checks if rewrite succeeded and fits on a single line.
let accept_rewrite = rewrite.as_ref().map(|result| !result.contains('\n')).unwrap_or(false);
Expand Down Expand Up @@ -263,7 +264,8 @@ impl Rewrite for ast::Block {

if !trimmed.is_empty() {
// 9 = "unsafe {".len(), 7 = "unsafe ".len()
format!("unsafe {} ", rewrite_comment(trimmed, true, width - 9, offset + 7))
let budget = try_opt!(width.checked_sub(9));
format!("unsafe {} ", rewrite_comment(trimmed, true, budget, offset + 7))
} else {
"unsafe ".to_owned()
}
Expand Down Expand Up @@ -357,7 +359,7 @@ impl<'a> Rewrite for Loop<'a> {
fn rewrite(&self, context: &RewriteContext, width: usize, offset: usize) -> Option<String> {
let label_string = rewrite_label(self.label);
// 2 = " {".len()
let inner_width = width - self.keyword.len() - 2 - label_string.len();
let inner_width = try_opt!(width.checked_sub(self.keyword.len() + 2 + label_string.len()));
let inner_offset = offset + self.keyword.len() + label_string.len();

let pat_expr_string = match self.cond {
Expand Down Expand Up @@ -393,14 +395,17 @@ fn rewrite_range(context: &RewriteContext,
offset: usize)
-> Option<String> {
let left_string = match left {
Some(expr) => try_opt!(expr.rewrite(context, width - 2, offset)),
Some(expr) => {
// 2 = ..
let max_width = try_opt!(width.checked_sub(2));
try_opt!(expr.rewrite(context, max_width, offset))
}
None => String::new(),
};

let right_string = match right {
Some(expr) => {
// 2 = ..
let max_width = (width - 2).checked_sub(left_string.len()).unwrap_or(0);
let max_width = try_opt!(width.checked_sub(left_string.len() + 2));
try_opt!(expr.rewrite(context, max_width, offset + 2 + left_string.len()))
}
None => String::new(),
Expand Down Expand Up @@ -532,7 +537,8 @@ fn rewrite_match(context: &RewriteContext,
}

// `match `cond` {`
let cond_str = try_opt!(cond.rewrite(context, width - 8, offset + 6));
let cond_budget = try_opt!(width.checked_sub(8));
let cond_str = try_opt!(cond.rewrite(context, cond_budget, offset + 6));
let mut result = format!("match {} {{", cond_str);

let block_indent = context.block_indent;
Expand Down Expand Up @@ -632,17 +638,20 @@ impl Rewrite for ast::Arm {
};

// Patterns
let pat_strs = try_opt!(pats.iter().map(|p| p.rewrite(context,
// 5 = ` => {`
width - 5,
offset + context.config.tab_spaces))
// 5 = ` => {`
let pat_budget = try_opt!(width.checked_sub(5));
let pat_strs = try_opt!(pats.iter().map(|p| {
p.rewrite(context,
pat_budget,
offset + context.config.tab_spaces)
})
.collect::<Option<Vec<_>>>());

let mut total_width = pat_strs.iter().fold(0, |a, p| a + p.len());
// Add ` | `.len().
total_width += (pat_strs.len() - 1) * 3;

let mut vertical = total_width > width - 5 || pat_strs.iter().any(|p| p.contains('\n'));
let mut vertical = total_width > pat_budget || pat_strs.iter().any(|p| p.contains('\n'));
if !vertical {
// If the patterns were previously stacked, keep them stacked.
// FIXME should be an option.
Expand Down Expand Up @@ -710,9 +719,8 @@ impl Rewrite for ast::Arm {
return None;
}

let body_str = try_opt!(body.rewrite(context,
width - context.config.tab_spaces,
nested_indent));
let body_budget = try_opt!(width.checked_sub(context.config.tab_spaces));
let body_str = try_opt!(body.rewrite(context, body_budget, nested_indent));
Some(format!("{}{} =>\n{}{},",
attr_str.trim_left(),
pats_str,
Expand Down Expand Up @@ -775,9 +783,8 @@ fn rewrite_pat_expr(context: &RewriteContext,
let pat_offset = offset + matcher.len();
let mut result = match pat {
Some(pat) => {
let pat_string = try_opt!(pat.rewrite(context,
width - connector.len() - matcher.len(),
pat_offset));
let pat_budget = try_opt!(width.checked_sub(connector.len() + matcher.len()));
let pat_string = try_opt!(pat.rewrite(context, pat_budget, pat_offset));
format!("{}{}{}", matcher, pat_string, connector)
}
None => String::new(),
Expand Down Expand Up @@ -930,7 +937,8 @@ fn rewrite_struct_lit<'a>(context: &RewriteContext,
}

// 2 = " {".len()
let path_str = try_opt!(path.rewrite(context, width - 2, offset));
let path_budget = try_opt!(width.checked_sub(2));
let path_str = try_opt!(path.rewrite(context, path_budget, offset));

// Foo { a: Foo } - indent is +3, width is -5.
let h_budget = try_opt!(width.checked_sub(path_str.len() + 5));
Expand Down Expand Up @@ -1041,7 +1049,8 @@ fn rewrite_tuple_lit(context: &RewriteContext,
// In case of length 1, need a trailing comma
if items.len() == 1 {
// 3 = "(" + ",)"
return items[0].rewrite(context, width - 3, indent).map(|s| format!("({},)", s));
let budget = try_opt!(width.checked_sub(3));
return items[0].rewrite(context, budget, indent).map(|s| format!("({},)", s));
}

let items = itemize_list(context.codemap,
Expand All @@ -1057,7 +1066,8 @@ fn rewrite_tuple_lit(context: &RewriteContext,
span.lo + BytePos(1), // Remove parens
span.hi - BytePos(1));

let fmt = ListFormatting::for_fn(width - 2, indent);
let budget = try_opt!(width.checked_sub(2));
let fmt = ListFormatting::for_fn(budget, indent);

Some(format!("({})", write_list(&items.collect::<Vec<_>>(), &fmt)))
}
Expand Down Expand Up @@ -1134,11 +1144,10 @@ fn rewrite_unary_op(context: &RewriteContext,
ast::UnOp::UnNot => "!",
ast::UnOp::UnNeg => "-",
};
let operator_len = operator_str.len();

let subexpr =
try_opt!(expr.rewrite(context, try_opt!(width.checked_sub(operator_str.len())), offset));

Some(format!("{}{}", operator_str, subexpr))
expr.rewrite(context, try_opt!(width.checked_sub(operator_len)), offset + operator_len)
.map(|r| format!("{}{}", operator_str, r))
}

fn rewrite_assignment(context: &RewriteContext,
Expand Down
6 changes: 6 additions & 0 deletions tests/source/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,9 @@ fn qux() {
// A block with a comment.
}
}

fn issue227() {
{
let handler = box DocumentProgressHandler::new(addr, DocumentProgressTask::DOMContentLoaded);
}
}
7 changes: 7 additions & 0 deletions tests/target/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,3 +139,10 @@ fn qux() {
// A block with a comment.
}
}

fn issue227() {
{
let handler = box DocumentProgressHandler::new(addr,
DocumentProgressTask::DOMContentLoaded);
}
}