Skip to content
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

Bump rustfmt version #80843

Merged
merged 1 commit into from
Feb 2, 2021
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
4 changes: 3 additions & 1 deletion compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let count = generics
.params
.iter()
.filter(|param| matches!(param.kind, ast::GenericParamKind::Lifetime { .. }))
.filter(|param| {
matches!(param.kind, ast::GenericParamKind::Lifetime { .. })
})
.count();
self.lctx.type_def_lifetime_params.insert(def_id.to_def_id(), count);
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ impl<'a> TraitDef<'a> {

let mut ty_params = params
.iter()
.filter(|param| matches!(param.kind, ast::GenericParamKind::Type{..}))
.filter(|param| matches!(param.kind, ast::GenericParamKind::Type { .. }))
.peekable();

if ty_params.peek().is_some() {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_errors/src/diagnostic_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ macro_rules! forward_inner_docs {
($e:expr => $i:item) => {
#[doc = $e]
$i
}
};
}

/// In general, the `DiagnosticBuilder` uses deref to allow access to
Expand Down
15 changes: 10 additions & 5 deletions compiler/rustc_errors/src/snippet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,13 @@ impl Annotation {
}

pub fn is_multiline(&self) -> bool {
matches!(self.annotation_type,
matches!(
self.annotation_type,
AnnotationType::Multiline(_)
| AnnotationType::MultilineStart(_)
| AnnotationType::MultilineLine(_)
| AnnotationType::MultilineEnd(_))
| AnnotationType::MultilineStart(_)
| AnnotationType::MultilineLine(_)
| AnnotationType::MultilineEnd(_)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@calebcartwright
This looks like a regression, | patterns outside of matches! are not formatted with an indent like this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one requires a much more thorough response. The short answer is that no it's not a regression, though you're not the first person to raise the question and I can certainly understand why.

For the longer answer, there's two core parts.

First, there was an issue in rustfmt where in cases with multi pats and at least one _ rustfmt was entirely unable to process the arg and thus wasn't applying any formatting at all against the mac call expr and just leaving the original formatting in place; rustfmt was just keeping whatever was here before as-is, not successfully applying the formatting rules.

An easy way to see this in practice is by running the older versions (I guess x.py fmt from master will work) with a wildly misformatted version of this, and you'll see that wild formatting remains untouched:

        matches!(self.annotation_type,AnnotationType::Multiline(_)
| AnnotationType::MultilineStart(_)                      | AnnotationType::MultilineLine(_)
              | AnnotationType::MultilineEnd(_)
        )

On the rustfmt side we've been particularly benefiting from the recent parser/tokens/etc. work that folks like yourself and Aaron have been working on, and now that we've pulled those rustc changes into rustfmt it's finally able to actually format these matches! calls producing the formatting seen here.

The second part relates to how we have to attempt to format macro call args in rustfmt. Obviously we're working with the calls in the AST pre-expansion, so when we encounter a maccall expression, we'll use the token streams to instantiate a parser and attempt to parse the args individually so that we can then apply the corresponding formatting rules when possible (e.g. an expr or type).

In the case of matches!, the tokens for that second arg actually ends up getting successfully parsed as a standard binop expression, and so rustfmt applies the corresponding formatting rules for binops which is why it gets indented this way.

Eventually I'd like to introduce some new special case process for matches! mac calls since we'll know definitively that second arg isn't actually a binop expr, and then be able to apply more pattern-like formatting rules instead.

Unfortunately though, for the foreseeable future rustfmt is going to continue to be formatting matches like any other mac call even though the emitted formatting is less than desireable

)
}

pub fn len(&self) -> usize {
Expand Down Expand Up @@ -158,7 +160,10 @@ impl Annotation {

pub fn takes_space(&self) -> bool {
// Multiline annotations always have to keep vertical space.
matches!(self.annotation_type, AnnotationType::MultilineStart(_) | AnnotationType::MultilineEnd(_))
matches!(
self.annotation_type,
AnnotationType::MultilineStart(_) | AnnotationType::MultilineEnd(_)
)
}
}

Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1543,10 +1543,10 @@ pub fn is_range_literal(expr: &Expr<'_>) -> bool {
**qpath,
QPath::LangItem(
LangItem::Range
| LangItem::RangeTo
| LangItem::RangeFrom
| LangItem::RangeFull
| LangItem::RangeToInclusive,
| LangItem::RangeTo
| LangItem::RangeFrom
| LangItem::RangeFull
| LangItem::RangeToInclusive,
_,
)
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,12 @@ impl Visitor<'tcx> for TypeParamSpanVisitor<'tcx> {
[segment]
if segment
.res
.map(|res| matches!(res, Res::SelfTy(_, _) | Res::Def(hir::def::DefKind::TyParam, _)))
.map(|res| {
matches!(
res,
Res::SelfTy(_, _) | Res::Def(hir::def::DefKind::TyParam, _)
)
})
.unwrap_or(false) =>
{
self.types.push(path.span);
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_middle/src/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -569,9 +569,9 @@ impl<'hir> Map<'hir> {
self.find(self.get_parent_node(id)),
Some(
Node::Item(_)
| Node::TraitItem(_)
| Node::ImplItem(_)
| Node::Expr(Expr { kind: ExprKind::Closure(..), .. }),
| Node::TraitItem(_)
| Node::ImplItem(_)
| Node::Expr(Expr { kind: ExprKind::Closure(..), .. }),
)
)
}
Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -962,8 +962,7 @@ impl<'tcx> LocalDecl<'tcx> {
opt_ty_info: _,
opt_match_place: _,
pat_span: _,
})
| BindingForm::ImplicitSelf(ImplicitSelfKind::Imm),
}) | BindingForm::ImplicitSelf(ImplicitSelfKind::Imm),
)))
)
}
Expand All @@ -980,8 +979,7 @@ impl<'tcx> LocalDecl<'tcx> {
opt_ty_info: _,
opt_match_place: _,
pat_span: _,
})
| BindingForm::ImplicitSelf(_),
}) | BindingForm::ImplicitSelf(_),
)))
)
}
Expand Down
18 changes: 11 additions & 7 deletions compiler/rustc_middle/src/ty/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,17 @@ impl<'tcx> TyS<'tcx> {
pub fn is_primitive_ty(&self) -> bool {
matches!(
self.kind(),
Bool | Char | Str | Int(_) | Uint(_) | Float(_)
| Infer(
InferTy::IntVar(_)
| InferTy::FloatVar(_)
| InferTy::FreshIntTy(_)
| InferTy::FreshFloatTy(_)
)
Bool | Char
| Str
| Int(_)
| Uint(_)
| Float(_)
| Infer(
InferTy::IntVar(_)
| InferTy::FloatVar(_)
| InferTy::FreshIntTy(_)
| InferTy::FreshFloatTy(_)
)
)
}

Expand Down
11 changes: 7 additions & 4 deletions compiler/rustc_middle/src/ty/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -646,11 +646,14 @@ impl<T> Trait<T> for X {
let current_method_ident = body_owner.and_then(|n| n.ident()).map(|i| i.name);

// We don't want to suggest calling an assoc fn in a scope where that isn't feasible.
let callable_scope = matches!(body_owner, Some(
let callable_scope = matches!(
body_owner,
Some(
hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(..), .. })
| hir::Node::TraitItem(hir::TraitItem { kind: hir::TraitItemKind::Fn(..), .. })
| hir::Node::ImplItem(hir::ImplItem { kind: hir::ImplItemKind::Fn(..), .. }),
));
| hir::Node::TraitItem(hir::TraitItem { kind: hir::TraitItemKind::Fn(..), .. })
| hir::Node::ImplItem(hir::ImplItem { kind: hir::ImplItemKind::Fn(..), .. }),
)
);
let impl_comparison = matches!(
cause_code,
ObligationCauseCode::CompareImplMethodObligation { .. }
Expand Down
10 changes: 8 additions & 2 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1871,8 +1871,14 @@ impl<'tcx> TyS<'tcx> {
pub fn is_scalar(&self) -> bool {
matches!(
self.kind(),
Bool | Char | Int(_) | Float(_) | Uint(_) | FnDef(..) | FnPtr(_) | RawPtr(_)
| Infer(IntVar(_) | FloatVar(_))
Bool | Char
| Int(_)
| Float(_)
| Uint(_)
| FnDef(..)
| FnPtr(_)
| RawPtr(_)
| Infer(IntVar(_) | FloatVar(_))
)
}

Expand Down
86 changes: 46 additions & 40 deletions compiler/rustc_mir_build/src/build/expr/as_place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use crate::build::{BlockAnd, BlockAndExtension, Builder};
use crate::thir::*;
use rustc_hir::def_id::DefId;
use rustc_hir::HirId;
use rustc_middle::middle::region;
use rustc_middle::hir::place::ProjectionKind as HirProjectionKind;
use rustc_middle::middle::region;
use rustc_middle::mir::AssertKind::BoundsCheck;
use rustc_middle::mir::*;
use rustc_middle::ty::{self, CanonicalUserTypeAnnotation, Ty, TyCtxt, Variance};
Expand Down Expand Up @@ -57,7 +57,8 @@ crate enum PlaceBase {
/// DefId of the closure
closure_def_id: DefId,
/// The trait closure implements, `Fn`, `FnMut`, `FnOnce`
closure_kind: ty::ClosureKind },
closure_kind: ty::ClosureKind,
},
}

/// `PlaceBuilder` is used to create places during MIR construction. It allows you to "build up" a
Expand All @@ -81,8 +82,7 @@ crate struct PlaceBuilder<'tcx> {
fn convert_to_hir_projections_and_truncate_for_capture<'tcx>(
mir_projections: &[PlaceElem<'tcx>],
) -> Vec<HirProjectionKind> {

let mut hir_projections = Vec::new();
let mut hir_projections = Vec::new();

for mir_projection in mir_projections {
let hir_projection = match mir_projection {
Expand All @@ -91,20 +91,20 @@ fn convert_to_hir_projections_and_truncate_for_capture<'tcx>(
// We will never encouter this for multivariant enums,
// read the comment for `Downcast`.
HirProjectionKind::Field(field.index() as u32, VariantIdx::new(0))
},
}
ProjectionElem::Downcast(..) => {
// This projections exist only for enums that have
// multiple variants. Since such enums that are captured
// completely, we can stop here.
break
},
break;
}
ProjectionElem::Index(..)
| ProjectionElem::ConstantIndex { .. }
| ProjectionElem::Subslice { .. } => {
// We don't capture array-access projections.
// We can stop here as arrays are captured completely.
break
},
break;
}
};

hir_projections.push(hir_projection);
Expand Down Expand Up @@ -181,9 +181,9 @@ fn find_capture_matching_projections<'a, 'tcx>(
// If an ancestor is found, `idx` is the index within the list of captured places
// for root variable `var_hir_id` and `capture` is the `ty::CapturedPlace` itself.
let (idx, capture) = root_variable_min_captures.iter().enumerate().find(|(_, capture)| {
let possible_ancestor_proj_kinds =
capture.place.projections.iter().map(|proj| proj.kind).collect();
is_ancestor_or_same_capture(&possible_ancestor_proj_kinds, &hir_projections)
let possible_ancestor_proj_kinds =
capture.place.projections.iter().map(|proj| proj.kind).collect();
is_ancestor_or_same_capture(&possible_ancestor_proj_kinds, &hir_projections)
})?;

// Convert index to be from the presepective of the entire closure_min_captures map
Expand Down Expand Up @@ -213,35 +213,34 @@ fn to_upvars_resolved_place_builder<'a, 'tcx>(
ty::ClosureKind::FnOnce => {}
}

let (capture_index, capture) =
if let Some(capture_details) = find_capture_matching_projections(
let (capture_index, capture) = if let Some(capture_details) =
find_capture_matching_projections(
typeck_results,
var_hir_id,
closure_def_id,
&from_builder.projection,
) {
capture_details
} else {
if !tcx.features().capture_disjoint_fields {
bug!(
"No associated capture found for {:?}[{:#?}] even though \
capture_details
} else {
if !tcx.features().capture_disjoint_fields {
bug!(
"No associated capture found for {:?}[{:#?}] even though \
capture_disjoint_fields isn't enabled",
var_hir_id,
from_builder.projection
)
} else {
// FIXME(project-rfc-2229#24): Handle this case properly
debug!(
"No associated capture found for {:?}[{:#?}]",
var_hir_id,
from_builder.projection,
);
}
return Err(var_hir_id);
};
var_hir_id,
from_builder.projection
)
} else {
// FIXME(project-rfc-2229#24): Handle this case properly
debug!(
"No associated capture found for {:?}[{:#?}]",
var_hir_id, from_builder.projection,
);
}
return Err(var_hir_id);
};

let closure_ty =
typeck_results.node_type(tcx.hir().local_def_id_to_hir_id(closure_def_id.expect_local()));
let closure_ty = typeck_results
.node_type(tcx.hir().local_def_id_to_hir_id(closure_def_id.expect_local()));

let substs = match closure_ty.kind() {
ty::Closure(_, substs) => ty::UpvarSubsts::Closure(substs),
Expand All @@ -256,7 +255,8 @@ fn to_upvars_resolved_place_builder<'a, 'tcx>(
// we know that the capture exists and is the `capture_index`-th capture.
let var_ty = substs.tupled_upvars_ty().tuple_element_ty(capture_index).unwrap();

upvar_resolved_place_builder = upvar_resolved_place_builder.field(Field::new(capture_index), var_ty);
upvar_resolved_place_builder =
upvar_resolved_place_builder.field(Field::new(capture_index), var_ty);

// If the variable is captured via ByRef(Immutable/Mutable) Borrow,
// we need to deref it
Expand All @@ -270,8 +270,9 @@ fn to_upvars_resolved_place_builder<'a, 'tcx>(

// We used some of the projections to build the capture itself,
// now we apply the remaining to the upvar resolved place.
upvar_resolved_place_builder.projection.extend(
curr_projections.drain(next_projection..));
upvar_resolved_place_builder
.projection
.extend(curr_projections.drain(next_projection..));

Ok(upvar_resolved_place_builder)
}
Expand Down Expand Up @@ -356,7 +357,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

/// This is used when constructing a compound `Place`, so that we can avoid creating
/// intermediate `Place` values until we know the full set of projections.
crate fn as_place_builder<M>(&mut self, block: BasicBlock, expr: M) -> BlockAnd<PlaceBuilder<'tcx>>
crate fn as_place_builder<M>(
&mut self,
block: BasicBlock,
expr: M,
) -> BlockAnd<PlaceBuilder<'tcx>>
where
M: Mirror<'tcx, Output = Expr<'tcx>>,
{
Expand Down Expand Up @@ -627,7 +632,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
if is_outermost_index {
self.read_fake_borrows(block, fake_borrow_temps, source_info)
} else {
base_place = base_place.expect_upvars_resolved(self.hir.tcx(), self.hir.typeck_results());
base_place =
base_place.expect_upvars_resolved(self.hir.tcx(), self.hir.typeck_results());
self.add_fake_borrows_of_base(
&base_place,
block,
Expand Down Expand Up @@ -679,7 +685,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let tcx = self.hir.tcx();
let local = match base_place.base {
PlaceBase::Local(local) => local,
PlaceBase::Upvar { .. } => bug!("Expected PlacseBase::Local found Upvar")
PlaceBase::Upvar { .. } => bug!("Expected PlacseBase::Local found Upvar"),
};

let place_ty = Place::ty_from(local, &base_place.projection, &self.local_decls, tcx);
Expand Down
Loading