Skip to content

Commit

Permalink
[unused_braces] Lint multiline blocks as long as not in arms
Browse files Browse the repository at this point in the history
Currently the lint faces a severe limitation: since it only catches single-line block, running rustfmt beforehand will remove all occurences of it, because it breaks them into multiline blocks.

We do not check match `Arm` for two reasons:
- In case it does not use commas to separate arms, removing the block would result in a compilation error
Example:
```
match expr {
    pat => {()}
    _ => println!("foo")
}
```
- Do not lint multiline match arms used for formatting reasons
```
match expr {
   pat => {
       somewhat_long_expression
   }
 // ...
}

Delete `unused-braces-lint` test

The modified lint correctly provide a span in its suggestion.

```shell
error: unnecessary braces around block return value
  --> /rust/src/test/rustdoc-ui/unused-braces-lint.rs:9:5
   |
LL | /     {
LL | |         {
   | |________^
LL |               use std;
LL |           }
   |  __________^
LL | |     }
   | |_____^
   |
note: the lint level is defined here
  --> /rust/src/test/rustdoc-ui/unused-braces-lint.rs:6:9
   |
LL | #![deny(unused_braces)]
   |         ^^^^^^^^^^^^^
help: remove these braces
   |
LL ~     {
LL |             use std;
LL ~         }
   |
```

It is unclear to which extend rust-lang#70814 is still an issue, as the inital MCVE does not trigger the lint on stable
 either,[rust playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=b6ff31a449c0b73a08daac8ee43b1fa6)

Fix code with expanded `unused_braces` lint

Also allow `unused_braces` on tests
  • Loading branch information
kraktus committed Jan 13, 2023
1 parent 0b90256 commit 9d3e6ba
Show file tree
Hide file tree
Showing 39 changed files with 250 additions and 273 deletions.
12 changes: 6 additions & 6 deletions compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ pub(crate) fn codegen_fn<'tcx>(
let mir = tcx.instance_mir(instance.def);
let _mir_guard = crate::PrintOnPanic(|| {
let mut buf = Vec::new();
with_no_trimmed_paths!({
with_no_trimmed_paths!(
rustc_middle::mir::pretty::write_mir_fn(tcx, mir, &mut |_, _| Ok(()), &mut buf)
.unwrap();
});
.unwrap()
);
String::from_utf8_lossy(&buf).into_owned()
});

Expand Down Expand Up @@ -293,9 +293,9 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) {

if fx.clif_comments.enabled() {
let mut terminator_head = "\n".to_string();
with_no_trimmed_paths!({
bb_data.terminator().kind.fmt_head(&mut terminator_head).unwrap();
});
with_no_trimmed_paths!(
bb_data.terminator().kind.fmt_head(&mut terminator_head).unwrap()
);
let inst = fx.bcx.func.layout.last_inst(block).unwrap();
fx.add_comment(inst, terminator_head);
}
Expand Down
45 changes: 18 additions & 27 deletions compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -649,43 +649,34 @@ fn codegen_regular_intrinsic_call<'tcx>(

let layout = fx.layout_of(substs.type_at(0));
if layout.abi.is_uninhabited() {
with_no_trimmed_paths!({
crate::base::codegen_panic(
fx,
&format!("attempted to instantiate uninhabited type `{}`", layout.ty),
source_info,
)
});
with_no_trimmed_paths!(crate::base::codegen_panic(
fx,
&format!("attempted to instantiate uninhabited type `{}`", layout.ty),
source_info,
));
return;
}

if intrinsic == sym::assert_zero_valid && !fx.tcx.permits_zero_init(layout) {
with_no_trimmed_paths!({
crate::base::codegen_panic(
fx,
&format!(
"attempted to zero-initialize type `{}`, which is invalid",
layout.ty
),
source_info,
);
});
with_no_trimmed_paths!(crate::base::codegen_panic(
fx,
&format!("attempted to zero-initialize type `{}`, which is invalid", layout.ty),
source_info,
));
return;
}

if intrinsic == sym::assert_mem_uninitialized_valid
&& !fx.tcx.permits_uninit_init(layout)
{
with_no_trimmed_paths!({
crate::base::codegen_panic(
fx,
&format!(
"attempted to leave type `{}` uninitialized, which is invalid",
layout.ty
),
source_info,
)
});
with_no_trimmed_paths!(crate::base::codegen_panic(
fx,
&format!(
"attempted to leave type `{}` uninitialized, which is invalid",
layout.ty
),
source_info,
));
return;
}
}
Expand Down
32 changes: 15 additions & 17 deletions compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,27 +465,25 @@ pub fn type_di_node<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, t: Ty<'tcx>) -> &'ll D
_ => bug!("debuginfo: unexpected type in type_di_node(): {:?}", t),
};

{
if already_stored_in_typemap {
// Make sure that we really do have a `TypeMap` entry for the unique type ID.
let di_node_for_uid =
match debug_context(cx).type_map.di_node_for_unique_id(unique_type_id) {
Some(di_node) => di_node,
None => {
bug!(
"expected type debuginfo node for unique \
if already_stored_in_typemap {
// Make sure that we really do have a `TypeMap` entry for the unique type ID.
let di_node_for_uid = match debug_context(cx).type_map.di_node_for_unique_id(unique_type_id)
{
Some(di_node) => di_node,
None => {
bug!(
"expected type debuginfo node for unique \
type ID '{:?}' to already be in \
the `debuginfo::TypeMap` but it \
was not.",
unique_type_id,
);
}
};
unique_type_id,
);
}
};

debug_assert_eq!(di_node_for_uid as *const _, di_node as *const _);
} else {
debug_context(cx).type_map.insert(unique_type_id, di_node);
}
debug_assert_eq!(di_node_for_uid as *const _, di_node as *const _);
} else {
debug_context(cx).type_map.insert(unique_type_id, di_node);
}

di_node
Expand Down
26 changes: 11 additions & 15 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -682,21 +682,17 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
MemUninitializedValid => !bx.tcx().permits_uninit_init(layout),
};
Some(if do_panic {
let msg_str = with_no_visible_paths!({
with_no_trimmed_paths!({
if layout.abi.is_uninhabited() {
// Use this error even for the other intrinsics as it is more precise.
format!("attempted to instantiate uninhabited type `{}`", ty)
} else if intrinsic == ZeroValid {
format!("attempted to zero-initialize type `{}`, which is invalid", ty)
} else {
format!(
"attempted to leave type `{}` uninitialized, which is invalid",
ty
)
}
})
});
let msg_str = with_no_visible_paths!(with_no_trimmed_paths!(if layout
.abi
.is_uninhabited()
{
// Use this error even for the other intrinsics as it is more precise.
format!("attempted to instantiate uninhabited type `{}`", ty)
} else if intrinsic == ZeroValid {
format!("attempted to zero-initialize type `{}`, which is invalid", ty)
} else {
format!("attempted to leave type `{}` uninitialized, which is invalid", ty)
}));
let msg = bx.const_str(&msg_str);

// Obtain the panic entry point.
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_expand/src/placeholders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,17 +144,17 @@ pub fn placeholder(
span,
is_placeholder: true,
}]),
AstFragmentKind::GenericParams => AstFragment::GenericParams(smallvec![{
ast::GenericParam {
AstFragmentKind::GenericParams => {
AstFragment::GenericParams(smallvec![ast::GenericParam {
attrs: Default::default(),
bounds: Default::default(),
id,
ident,
is_placeholder: true,
kind: ast::GenericParamKind::Lifetime,
colon_span: None,
}
}]),
}])
}
AstFragmentKind::Params => AstFragment::Params(smallvec![ast::Param {
attrs: Default::default(),
id,
Expand Down
10 changes: 4 additions & 6 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1194,12 +1194,10 @@ impl<'tcx> LateContext<'tcx> {
}

// This shouldn't ever be needed, but just in case:
with_no_trimmed_paths!({
Ok(vec![match trait_ref {
Some(trait_ref) => Symbol::intern(&format!("{:?}", trait_ref)),
None => Symbol::intern(&format!("<{}>", self_ty)),
}])
})
with_no_trimmed_paths!(Ok(vec![match trait_ref {
Some(trait_ref) => Symbol::intern(&format!("{:?}", trait_ref)),
None => Symbol::intern(&format!("<{}>", self_ty)),
}]))
}

fn path_append_impl(
Expand Down
27 changes: 17 additions & 10 deletions compiler/rustc_lint/src/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1084,26 +1084,33 @@ impl UnusedDelimLint for UnusedBraces {
// - the block is not `unsafe`
// - the block contains exactly one expression (do not lint `{ expr; }`)
// - `followed_by_block` is true and the internal expr may contain a `{`
// - the block is not multiline (do not lint multiline match arms)
// ```
// match expr {
// Pattern => {
// somewhat_long_expression
// }
// // ...
// }
// ```
// - the block has no attribute and was not created inside a macro
// - if the block is an `anon_const`, the inner expr must be a literal
// (do not lint `struct A<const N: usize>; let _: A<{ 2 + 3 }>;`)
//
// We do not check expression in `Arm` bodies:
// - if not using commas to separate arms, removing the block would result in a compilation error
// ```
// match expr {
// pat => {()}
// _ => println!("foo")
// }
// ```
// - multiline blocks can used for formatting
// ```
// match expr {
// pat => {
// somewhat_long_expression
// }
// // ...
// }
// ```
// FIXME(const_generics): handle paths when #67075 is fixed.
if let [stmt] = inner.stmts.as_slice() {
if let ast::StmtKind::Expr(ref expr) = stmt.kind {
if !Self::is_expr_delims_necessary(expr, followed_by_block, false)
&& (ctx != UnusedDelimsCtx::AnonConst
|| matches!(expr.kind, ast::ExprKind::Lit(_)))
&& !cx.sess().source_map().is_multiline(value.span)
&& value.attrs.is_empty()
&& !value.span.from_expansion()
&& !inner.span.from_expansion()
Expand Down
9 changes: 5 additions & 4 deletions compiler/rustc_middle/src/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,27 +21,27 @@ use std::sync::Arc;
impl fmt::Debug for ty::TraitDef {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
ty::tls::with(|tcx| {
with_no_trimmed_paths!({
with_no_trimmed_paths!(
f.write_str(
&FmtPrinter::new(tcx, Namespace::TypeNS)
.print_def_path(self.def_id, &[])?
.into_buffer(),
)
})
)
})
}
}

impl<'tcx> fmt::Debug for ty::AdtDef<'tcx> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
ty::tls::with(|tcx| {
with_no_trimmed_paths!({
with_no_trimmed_paths!(
f.write_str(
&FmtPrinter::new(tcx, Namespace::TypeNS)
.print_def_path(self.did(), &[])?
.into_buffer(),
)
})
)
})
}
}
Expand Down Expand Up @@ -76,6 +76,7 @@ impl fmt::Debug for ty::BoundRegionKind {
write!(f, "BrNamed({:?}, {})", did, name)
}
}

ty::BrEnv => write!(f, "BrEnv"),
}
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,9 +476,9 @@ fn check_for_bindings_named_same_as_variants(
})
{
let variant_count = edef.variants().len();
let ty_path = with_no_trimmed_paths!({
let ty_path = with_no_trimmed_paths!(
cx.tcx.def_path_str(edef.did())
});
);
cx.tcx.emit_spanned_lint(
BINDINGS_WITH_VARIANT_NAME,
p.hir_id,
Expand Down
48 changes: 23 additions & 25 deletions compiler/rustc_save_analysis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -964,35 +964,33 @@ pub fn process_crate<H: SaveHandler>(
config: Option<Config>,
mut handler: H,
) {
with_no_trimmed_paths!({
tcx.dep_graph.with_ignore(|| {
info!("Dumping crate {}", cratename);

// Privacy checking must be done outside of type inference; use a
// fallback in case effective visibilities couldn't have been correctly computed.
let effective_visibilities = match tcx.sess.compile_status() {
Ok(..) => tcx.effective_visibilities(()),
Err(..) => tcx.arena.alloc(EffectiveVisibilities::default()),
};
with_no_trimmed_paths!(tcx.dep_graph.with_ignore(|| {
info!("Dumping crate {}", cratename);

// Privacy checking must be done outside of type inference; use a
// fallback in case effective visibilities couldn't have been correctly computed.
let effective_visibilities = match tcx.sess.compile_status() {
Ok(..) => tcx.effective_visibilities(()),
Err(..) => tcx.arena.alloc(EffectiveVisibilities::default()),
};

let save_ctxt = SaveContext {
tcx,
maybe_typeck_results: None,
effective_visibilities: &effective_visibilities,
span_utils: SpanUtils::new(&tcx.sess),
config: find_config(config),
impl_counter: Cell::new(0),
};
let save_ctxt = SaveContext {
tcx,
maybe_typeck_results: None,
effective_visibilities: &effective_visibilities,
span_utils: SpanUtils::new(&tcx.sess),
config: find_config(config),
impl_counter: Cell::new(0),
};

let mut visitor = DumpVisitor::new(save_ctxt);
let mut visitor = DumpVisitor::new(save_ctxt);

visitor.dump_crate_info(cratename);
visitor.dump_compilation_options(input, cratename);
visitor.process_crate();
visitor.dump_crate_info(cratename);
visitor.dump_compilation_options(input, cratename);
visitor.process_crate();

handler.save(&visitor.save_ctxt, &visitor.analysis())
})
})
handler.save(&visitor.save_ctxt, &visitor.analysis())
}))
}

fn find_config(supplied: Option<Config>) -> Config {
Expand Down
8 changes: 3 additions & 5 deletions compiler/rustc_span/src/hygiene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1332,11 +1332,9 @@ pub fn decode_syntax_context<D: Decoder, F: FnOnce(&mut D, u32) -> SyntaxContext

let outer_ctxts = &context.remapped_ctxts;

// Ensure that the lock() temporary is dropped early
{
if let Some(ctxt) = outer_ctxts.lock().get(raw_id as usize).copied().flatten() {
return ctxt;
}
// the lock() temporary is dropped early
if let Some(ctxt) = outer_ctxts.lock().get(raw_id as usize).copied().flatten() {
return ctxt;
}

// Allocate and store SyntaxContext id *before* calling the decoder function,
Expand Down
16 changes: 9 additions & 7 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2055,13 +2055,15 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
ty::Adt(def, substs) => {
let sized_crit = def.sized_constraint(self.tcx());
// (*) binder moved here
Where(obligation.predicate.rebind({
sized_crit
.0
.iter()
.map(|ty| sized_crit.rebind(*ty).subst(self.tcx(), substs))
.collect()
}))
Where(
obligation.predicate.rebind(
sized_crit
.0
.iter()
.map(|ty| sized_crit.rebind(*ty).subst(self.tcx(), substs))
.collect(),
),
)
}

ty::Alias(..) | ty::Param(_) => None,
Expand Down
Loading

0 comments on commit 9d3e6ba

Please sign in to comment.