Skip to content

Commit 4288736

Browse files
bvanjoibvanjoi
committed
skip necessary paren expr during unused_paren check
1 parent 21fe748 commit 4288736

File tree

13 files changed

+181
-4
lines changed

13 files changed

+181
-4
lines changed

‎compiler/rustc_ast/src/mut_visit.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -807,7 +807,7 @@ fn visit_lazy_tts_opt_mut<T: MutVisitor>(vis: &mut T, lazy_tts: Option<&mut Lazy
807807
}
808808
}
809809

810-
fn visit_lazy_tts<T: MutVisitor>(vis: &mut T, lazy_tts: &mut Option<LazyAttrTokenStream>) {
810+
pub fn visit_lazy_tts<T: MutVisitor>(vis: &mut T, lazy_tts: &mut Option<LazyAttrTokenStream>) {
811811
visit_lazy_tts_opt_mut(vis, lazy_tts.as_mut());
812812
}
813813

‎compiler/rustc_expand/src/base.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1073,6 +1073,8 @@ pub trait ResolverExpand {
10731073
trait_def_id: DefId,
10741074
impl_def_id: LocalDefId,
10751075
) -> Result<Vec<(Ident, Option<Ident>)>, Indeterminate>;
1076+
1077+
fn add_necessary_parens(&mut self, node_span: Span);
10761078
}
10771079

10781080
pub trait LintStoreExpand {

‎compiler/rustc_expand/src/expand.rs

+39-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::ops::Deref;
1+
use std::ops::{Deref, DerefMut};
22
use std::path::PathBuf;
33
use std::rc::Rc;
44
use std::{iter, mem};
@@ -594,6 +594,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
594594
cx: self.cx,
595595
invocations: Vec::new(),
596596
monotonic: self.monotonic,
597+
expr_under_let_init_else: false,
597598
};
598599
fragment.mut_visit_with(&mut collector);
599600
fragment.add_placeholders(extra_placeholders);
@@ -1771,6 +1772,7 @@ struct InvocationCollector<'a, 'b> {
17711772
cx: &'a mut ExtCtxt<'b>,
17721773
invocations: Vec<(Invocation, Option<Lrc<SyntaxExtension>>)>,
17731774
monotonic: bool,
1775+
expr_under_let_init_else: bool,
17741776
}
17751777

17761778
impl<'a, 'b> InvocationCollector<'a, 'b> {
@@ -2167,6 +2169,35 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
21672169
self.flat_map_node(node)
21682170
}
21692171

2172+
fn visit_local(&mut self, local: &mut P<rustc_ast::Local>) {
2173+
let ast::Local { id, pat, ty, kind, span, colon_sp, attrs, tokens } = local.deref_mut();
2174+
self.visit_id(id);
2175+
for attr in attrs.iter_mut() {
2176+
self.visit_attribute(attr);
2177+
}
2178+
self.visit_pat(pat);
2179+
if let Some(ty) = ty {
2180+
self.visit_ty(ty);
2181+
}
2182+
match kind {
2183+
ast::LocalKind::Decl => {}
2184+
ast::LocalKind::Init(init) => {
2185+
self.visit_expr(init);
2186+
}
2187+
ast::LocalKind::InitElse(init, els) => {
2188+
self.expr_under_let_init_else = true;
2189+
self.visit_expr(init);
2190+
self.expr_under_let_init_else = false;
2191+
self.visit_block(els);
2192+
}
2193+
}
2194+
visit_lazy_tts(self, tokens);
2195+
if let Some(sp) = colon_sp {
2196+
self.visit_span(sp);
2197+
}
2198+
self.visit_span(span);
2199+
}
2200+
21702201
fn visit_crate(&mut self, node: &mut ast::Crate) {
21712202
self.visit_node(node)
21722203
}
@@ -2180,6 +2211,13 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
21802211
}
21812212

21822213
fn visit_expr(&mut self, node: &mut P<ast::Expr>) {
2214+
if self.expr_under_let_init_else
2215+
&& let ast::ExprKind::Paren(paren) = &node.kind
2216+
&& let ast::ExprKind::MacCall(mac) = &paren.kind
2217+
&& mac.args.delim == Delimiter::Brace
2218+
{
2219+
self.cx.resolver.add_necessary_parens(node.span);
2220+
}
21832221
// FIXME: Feature gating is performed inconsistently between `Expr` and `OptExpr`.
21842222
if let Some(attr) = node.attrs.first() {
21852223
self.cfg().maybe_emit_expr_attr_err(attr);

‎compiler/rustc_interface/src/passes.rs

+2
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ fn pre_expansion_lint<'a>(
8181
lint_store,
8282
registered_tools,
8383
None,
84+
None,
8485
rustc_lint::BuiltinCombinedPreExpansionLintPass::new(),
8586
check_node,
8687
);
@@ -315,6 +316,7 @@ fn early_lint_checks(tcx: TyCtxt<'_>, (): ()) {
315316
lint_store,
316317
tcx.registered_tools(()),
317318
Some(lint_buffer),
319+
Some(&resolver.necessary_parens),
318320
rustc_lint::BuiltinCombinedEarlyLintPass::new(),
319321
(&**krate, &*krate.attrs),
320322
)

‎compiler/rustc_lint/src/context.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
use std::cell::Cell;
77
use std::{iter, slice};
88

9-
use rustc_data_structures::fx::FxIndexMap;
9+
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
1010
use rustc_data_structures::sync;
1111
use rustc_data_structures::unord::UnordMap;
1212
use rustc_errors::{Diag, LintDiagnostic, MultiSpan};
@@ -509,6 +509,7 @@ pub struct LateContext<'tcx> {
509509
pub struct EarlyContext<'a> {
510510
pub builder: LintLevelsBuilder<'a, crate::levels::TopDown>,
511511
pub buffered: LintBuffer,
512+
pub necessary_parens: Option<&'a FxHashSet<Span>>,
512513
}
513514

514515
impl EarlyContext<'_> {
@@ -636,6 +637,7 @@ impl<'a> EarlyContext<'a> {
636637
lint_store: &'a LintStore,
637638
registered_tools: &'a RegisteredTools,
638639
buffered: LintBuffer,
640+
necessary_parens: Option<&'a FxHashSet<Span>>,
639641
) -> EarlyContext<'a> {
640642
EarlyContext {
641643
builder: LintLevelsBuilder::new(
@@ -646,6 +648,7 @@ impl<'a> EarlyContext<'a> {
646648
registered_tools,
647649
),
648650
buffered,
651+
necessary_parens,
649652
}
650653
}
651654
}

‎compiler/rustc_lint/src/early.rs

+3
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use rustc_ast::ptr::P;
88
use rustc_ast::visit::{self as ast_visit, Visitor, walk_list};
99
use rustc_ast::{self as ast, HasAttrs};
10+
use rustc_data_structures::fx::FxHashSet;
1011
use rustc_data_structures::stack::ensure_sufficient_stack;
1112
use rustc_feature::Features;
1213
use rustc_middle::ty::RegisteredTools;
@@ -364,6 +365,7 @@ pub fn check_ast_node<'a>(
364365
lint_store: &LintStore,
365366
registered_tools: &RegisteredTools,
366367
lint_buffer: Option<LintBuffer>,
368+
necessary_parens: Option<&'a FxHashSet<Span>>,
367369
builtin_lints: impl EarlyLintPass + 'static,
368370
check_node: impl EarlyCheckNode<'a>,
369371
) {
@@ -374,6 +376,7 @@ pub fn check_ast_node<'a>(
374376
lint_store,
375377
registered_tools,
376378
lint_buffer.unwrap_or_default(),
379+
necessary_parens,
377380
);
378381

379382
// Note: `passes` is often empty. In that case, it's faster to run

‎compiler/rustc_lint/src/unused.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -1060,7 +1060,8 @@ impl UnusedDelimLint for UnusedParens {
10601060
) {
10611061
match value.kind {
10621062
ast::ExprKind::Paren(ref inner) => {
1063-
if !Self::is_expr_delims_necessary(inner, ctx, followed_by_block)
1063+
if cx.necessary_parens.map_or(true, |set| !set.contains(&value.span))
1064+
&& !Self::is_expr_delims_necessary(inner, ctx, followed_by_block)
10641065
&& value.attrs.is_empty()
10651066
&& !value.span.from_expansion()
10661067
&& (ctx != UnusedDelimsCtx::LetScrutineeExpr

‎compiler/rustc_middle/src/ty/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,7 @@ pub struct ResolverAstLowering {
213213

214214
/// Information about functions signatures for delegation items expansion
215215
pub delegation_fn_sigs: LocalDefIdMap<DelegationFnSig>,
216+
pub necessary_parens: FxHashSet<Span>,
216217
}
217218

218219
#[derive(Debug)]

‎compiler/rustc_resolve/src/lib.rs

+8
Original file line numberDiff line numberDiff line change
@@ -1198,6 +1198,12 @@ pub struct Resolver<'ra, 'tcx> {
11981198
/// This is the `Span` where an `extern crate foo;` suggestion would be inserted, if `foo`
11991199
/// could be a crate that wasn't imported. For diagnostics use only.
12001200
current_crate_outer_attr_insert_span: Span,
1201+
1202+
/// It is used for the `unused_paren` lint because the lint checks after expansion.
1203+
/// This helps us record nodes that are wrapped by necessary parentheses.
1204+
///
1205+
/// The reason for using `Span` is that the id has not been generated yet.
1206+
pub necessary_parens: FxHashSet<Span>,
12011207
}
12021208

12031209
/// This provides memory for the rest of the crate. The `'ra` lifetime that is
@@ -1543,6 +1549,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
15431549
impl_unexpanded_invocations: Default::default(),
15441550
impl_binding_keys: Default::default(),
15451551
current_crate_outer_attr_insert_span,
1552+
necessary_parens: Default::default(),
15461553
};
15471554

15481555
let root_parent_scope = ParentScope::module(graph_root, &resolver);
@@ -1656,6 +1663,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
16561663
lifetime_elision_allowed: self.lifetime_elision_allowed,
16571664
lint_buffer: Steal::new(self.lint_buffer),
16581665
delegation_fn_sigs: self.delegation_fn_sigs,
1666+
necessary_parens: self.necessary_parens,
16591667
};
16601668
ResolverOutputs { global_ctxt, ast_lowering }
16611669
}

‎compiler/rustc_resolve/src/macros.rs

+4
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,10 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> {
534534
});
535535
Ok(idents)
536536
}
537+
538+
fn add_necessary_parens(&mut self, span: Span) {
539+
self.necessary_parens.insert(span);
540+
}
537541
}
538542

539543
impl<'ra, 'tcx> Resolver<'ra, 'tcx> {

‎tests/ui/lint/unused-parens-for-macro-call-with-brace.fixed

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//@ run-rustfix
2+
3+
#![deny(unused_parens)]
4+
5+
fn main() {
6+
macro_rules! x {
7+
() => { None::<i32> };
8+
}
9+
10+
let Some(_) = (x!{}) else { return }; // no error
11+
let Some(_) = (x!{}) else { return };
12+
//~^ ERROR: unnecessary parentheses around assigned value
13+
let Some(_) = (x!{}) else { return };
14+
//~^ ERROR: unnecessary parentheses around pattern
15+
let _ = x!{};
16+
let _ = x!{};
17+
//~^ ERROR: unnecessary parentheses around assigned value
18+
if let Some(_) = x!{} {};
19+
if let Some(_) = x!{} {};
20+
//~^ ERROR: unnecessary parentheses around `let` scrutinee expression
21+
while let Some(_) = x!{} {};
22+
while let Some(_) = x!{} {};
23+
//~^ ERROR: unnecessary parentheses around `let` scrutinee expression
24+
}

‎tests/ui/lint/unused-parens-for-macro-call-with-brace.rs

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//@ run-rustfix
2+
3+
#![deny(unused_parens)]
4+
5+
fn main() {
6+
macro_rules! x {
7+
() => { None::<i32> };
8+
}
9+
10+
let Some(_) = (x!{}) else { return }; // no error
11+
let Some(_) = ((x!{})) else { return };
12+
//~^ ERROR: unnecessary parentheses around assigned value
13+
let Some((_)) = (x!{}) else { return };
14+
//~^ ERROR: unnecessary parentheses around pattern
15+
let _ = x!{};
16+
let _ = (x!{});
17+
//~^ ERROR: unnecessary parentheses around assigned value
18+
if let Some(_) = x!{} {};
19+
if let Some(_) = (x!{}) {};
20+
//~^ ERROR: unnecessary parentheses around `let` scrutinee expression
21+
while let Some(_) = x!{} {};
22+
while let Some(_) = (x!{}) {};
23+
//~^ ERROR: unnecessary parentheses around `let` scrutinee expression
24+
}

‎tests/ui/lint/unused-parens-for-macro-call-with-brace.stderr

+67
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
error: unnecessary parentheses around assigned value
2+
--> $DIR/unused-parens-for-macro-call-with-brace.rs:11:19
3+
|
4+
LL | let Some(_) = ((x!{})) else { return };
5+
| ^ ^
6+
|
7+
note: the lint level is defined here
8+
--> $DIR/unused-parens-for-macro-call-with-brace.rs:3:9
9+
|
10+
LL | #![deny(unused_parens)]
11+
| ^^^^^^^^^^^^^
12+
help: remove these parentheses
13+
|
14+
LL - let Some(_) = ((x!{})) else { return };
15+
LL + let Some(_) = (x!{}) else { return };
16+
|
17+
18+
error: unnecessary parentheses around pattern
19+
--> $DIR/unused-parens-for-macro-call-with-brace.rs:13:14
20+
|
21+
LL | let Some((_)) = (x!{}) else { return };
22+
| ^ ^
23+
|
24+
help: remove these parentheses
25+
|
26+
LL - let Some((_)) = (x!{}) else { return };
27+
LL + let Some(_) = (x!{}) else { return };
28+
|
29+
30+
error: unnecessary parentheses around assigned value
31+
--> $DIR/unused-parens-for-macro-call-with-brace.rs:16:13
32+
|
33+
LL | let _ = (x!{});
34+
| ^ ^
35+
|
36+
help: remove these parentheses
37+
|
38+
LL - let _ = (x!{});
39+
LL + let _ = x!{};
40+
|
41+
42+
error: unnecessary parentheses around `let` scrutinee expression
43+
--> $DIR/unused-parens-for-macro-call-with-brace.rs:19:22
44+
|
45+
LL | if let Some(_) = (x!{}) {};
46+
| ^ ^
47+
|
48+
help: remove these parentheses
49+
|
50+
LL - if let Some(_) = (x!{}) {};
51+
LL + if let Some(_) = x!{} {};
52+
|
53+
54+
error: unnecessary parentheses around `let` scrutinee expression
55+
--> $DIR/unused-parens-for-macro-call-with-brace.rs:22:25
56+
|
57+
LL | while let Some(_) = (x!{}) {};
58+
| ^ ^
59+
|
60+
help: remove these parentheses
61+
|
62+
LL - while let Some(_) = (x!{}) {};
63+
LL + while let Some(_) = x!{} {};
64+
|
65+
66+
error: aborting due to 5 previous errors
67+

0 commit comments

Comments
 (0)