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 #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

Mute `unused_braces` on `match_ast!`
  • Loading branch information
kraktus committed Jan 27, 2023
1 parent 7919ef0 commit e641393
Show file tree
Hide file tree
Showing 32 changed files with 193 additions and 198 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 @@ -295,9 +295,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,45 +649,36 @@ 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_nounwind(
fx,
&format!("attempted to instantiate uninhabited type `{}`", layout.ty),
source_info,
)
});
with_no_trimmed_paths!(crate::base::codegen_panic_nounwind(
fx,
&format!("attempted to instantiate uninhabited type `{}`", layout.ty),
source_info,
));
return;
}

if intrinsic == sym::assert_zero_valid
&& !fx.tcx.permits_zero_init(fx.param_env().and(layout))
{
with_no_trimmed_paths!({
crate::base::codegen_panic_nounwind(
fx,
&format!(
"attempted to zero-initialize type `{}`, which is invalid",
layout.ty
),
source_info,
);
});
with_no_trimmed_paths!(crate::base::codegen_panic_nounwind(
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(fx.param_env().and(layout))
{
with_no_trimmed_paths!({
crate::base::codegen_panic_nounwind(
fx,
&format!(
"attempted to leave type `{}` uninitialized, which is invalid",
layout.ty
),
source_info,
)
});
with_no_trimmed_paths!(crate::base::codegen_panic_nounwind(
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 @@ -678,21 +678,17 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
MemUninitializedValid => !bx.tcx().permits_uninit_init(bx.param_env().and(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 @@ -1199,12 +1199,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
28 changes: 18 additions & 10 deletions compiler/rustc_lint/src/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1116,15 +1116,6 @@ 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
// not created by a macro, i.e. do not lint on:
Expand All @@ -1133,14 +1124,31 @@ impl UnusedDelimLint for UnusedBraces {
// let _: A<{ 2 + 3 }>;
// let _: A<{produces_literal!()}>;
// ```
//
// 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(_))
&& !expr.span.from_expansion()))
&& !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 @@ -22,27 +22,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 @@ -77,6 +77,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 @@ -488,9 +488,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
Loading

0 comments on commit e641393

Please sign in to comment.