Skip to content

Replace try_opt! macro with a ? operator #2035

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 3 commits into from
Oct 5, 2017
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
6 changes: 3 additions & 3 deletions Contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,13 +184,13 @@ places) is for the caller to shuffle around some of its other items to make
more width, then call the function again with more space.

Since it is common for callers to bail out when a callee fails, we often use a
`try_opt!` macro to make this pattern more succinct.
`?` operator to make this pattern more succinct.

One way we might find out that we don't have enough space is when computing how much
space we have. Something like `available_space = budget - overhead`. Since
widths are unsized integers, this would cause underflow. Therefore we use
checked subtraction: `available_space = try_opt!(budget.checked_sub(overhead))`.
`checked_sub` returns an `Option`, and if we would underflow `try_opt!` returns
checked subtraction: `available_space = budget.checked_sub(overhead)?`.
`checked_sub` returns an `Option`, and if we would underflow `?` returns
`None`, otherwise we proceed with the computed space.

Much syntax in Rust is lists: lists of arguments, lists of fields, lists of
Expand Down
38 changes: 18 additions & 20 deletions src/chains.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,9 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -
} else {
shape
};
let parent_rewrite = try_opt!(
parent
.rewrite(context, parent_shape)
.map(|parent_rw| parent_rw + &repeat_try(prefix_try_num))
);
let parent_rewrite = parent
.rewrite(context, parent_shape)
.map(|parent_rw| parent_rw + &repeat_try(prefix_try_num))?;
let parent_rewrite_contains_newline = parent_rewrite.contains('\n');
let is_small_parent = parent_rewrite.len() <= context.config.tab_spaces();

Expand Down Expand Up @@ -146,8 +144,8 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -
let overhead = last_line_width(&parent_rewrite);
let offset = parent_rewrite.lines().rev().next().unwrap().trim().len();
match context.config.chain_indent() {
IndentStyle::Visual => try_opt!(parent_shape.offset_left(overhead)),
IndentStyle::Block => try_opt!(parent_shape.block().offset_left(offset)),
IndentStyle::Visual => parent_shape.offset_left(overhead)?,
IndentStyle::Block => parent_shape.block().offset_left(offset)?,
}
} else {
other_child_shape
Expand All @@ -165,11 +163,9 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -
let last_subexpr = &subexpr_list[suffix_try_num];
let subexpr_list = &subexpr_list[suffix_try_num..subexpr_num - prefix_try_num];
let iter = subexpr_list.iter().skip(1).rev().zip(child_shape_iter);
let mut rewrites = try_opt!(
iter.map(|(e, shape)| {
rewrite_chain_subexpr(e, total_span, context, shape)
}).collect::<Option<Vec<_>>>()
);
let mut rewrites = iter.map(|(e, shape)| {
rewrite_chain_subexpr(e, total_span, context, shape)
}).collect::<Option<Vec<_>>>()?;

// Total of all items excluding the last.
let extend_last_subexr = last_line_extendable(&parent_rewrite) && rewrites.is_empty();
Expand All @@ -187,7 +183,7 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -
&& rewrites.iter().all(|s| !s.contains('\n'))
&& almost_total < one_line_budget;
let rewrite_last = || rewrite_chain_subexpr(last_subexpr, total_span, context, nested_shape);
let (last_subexpr_str, fits_single_line) = try_opt!(if all_in_one_line || extend_last_subexr {
let (last_subexpr_str, fits_single_line) = if all_in_one_line || extend_last_subexr {
parent_shape.offset_left(almost_total).map(|shape| {
if let Some(rw) = rewrite_chain_subexpr(last_subexpr, total_span, context, shape) {
let line_count = rw.lines().count();
Expand All @@ -207,11 +203,11 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -
} else {
(rewrite_last(), false)
}
})
})?
} else {
Some((rewrite_last(), false))
});
rewrites.push(try_opt!(last_subexpr_str));
(rewrite_last(), false)
};
rewrites.push(last_subexpr_str?);

let connector = if fits_single_line && !parent_rewrite_contains_newline {
// Yay, we can put everything on one line.
Expand Down Expand Up @@ -288,7 +284,7 @@ fn rewrite_try(
context: &RewriteContext,
shape: Shape,
) -> Option<String> {
let sub_expr = try_opt!(expr.rewrite(context, try_opt!(shape.sub_width(try_count))));
let sub_expr = expr.rewrite(context, shape.sub_width(try_count)?)?;
Some(format!("{}{}", sub_expr, repeat_try(try_count)))
}

Expand Down Expand Up @@ -472,8 +468,10 @@ fn rewrite_method_call(
let (lo, type_str) = if types.is_empty() {
(args[0].span.hi(), String::new())
} else {
let type_list: Vec<_> =
try_opt!(types.iter().map(|ty| ty.rewrite(context, shape)).collect());
let type_list = types
.iter()
.map(|ty| ty.rewrite(context, shape))
.collect::<Option<Vec<_>>>()?;

let type_str = if context.config.spaces_within_angle_brackets() && !type_list.is_empty() {
format!("::< {} >", type_list.join(", "))
Expand Down
22 changes: 8 additions & 14 deletions src/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ pub fn combine_strs_with_missing_comments(
last_line_width(prev_str) + first_line_width(next_str) + first_sep.len();

let indent_str = shape.indent.to_string(context.config);
let missing_comment = try_opt!(rewrite_missing_comment(span, shape, context));
let missing_comment = rewrite_missing_comment(span, shape, context)?;

if missing_comment.is_empty() {
if allow_extend && prev_str.len() + first_sep.len() + next_str.len() <= shape.width {
Expand Down Expand Up @@ -254,13 +254,7 @@ fn identify_comment(
.collect::<Vec<_>>()
.join("\n");

let first_group_str = try_opt!(rewrite_comment_inner(
&first_group,
block_style,
style,
shape,
config,
));
let first_group_str = rewrite_comment_inner(&first_group, block_style, style, shape, config)?;
if rest.is_empty() {
Some(first_group_str)
} else {
Expand Down Expand Up @@ -380,7 +374,7 @@ pub fn recover_missing_comment_in_span(
context: &RewriteContext,
used_width: usize,
) -> Option<String> {
let missing_comment = try_opt!(rewrite_missing_comment(span, shape, context));
let missing_comment = rewrite_missing_comment(span, shape, context)?;
if missing_comment.is_empty() {
Some(String::new())
} else {
Expand Down Expand Up @@ -610,7 +604,7 @@ where
type Item = (FullCodeCharKind, T::Item);

fn next(&mut self) -> Option<(FullCodeCharKind, T::Item)> {
let item = try_opt!(self.base.next());
let item = self.base.next()?;
let chr = item.get_char();
self.status = match self.status {
CharClassesStatus::LitString => match chr {
Expand Down Expand Up @@ -705,7 +699,7 @@ impl<'a> Iterator for UngroupedCommentCodeSlices<'a> {
type Item = (CodeCharKind, usize, &'a str);

fn next(&mut self) -> Option<Self::Item> {
let (kind, (start_idx, _)) = try_opt!(self.iter.next());
let (kind, (start_idx, _)) = self.iter.next()?;
match kind {
FullCodeCharKind::Normal => {
// Consume all the Normal code
Expand Down Expand Up @@ -886,14 +880,14 @@ impl<'a> Iterator for CommentReducer<'a> {
type Item = char;
fn next(&mut self) -> Option<Self::Item> {
loop {
let mut c = try_opt!(self.iter.next());
let mut c = self.iter.next()?;
if self.is_block && self.at_start_line {
while c.is_whitespace() {
c = try_opt!(self.iter.next());
c = self.iter.next()?;
}
// Ignore leading '*'
if c == '*' {
c = try_opt!(self.iter.next());
c = self.iter.next()?;
}
} else if c == '\n' {
self.at_start_line = true;
Expand Down
Loading