handle paren in macro expand for let-init-else expr

bvanjoi · bvanjoi · commit d4bc9e6261f4 · 2024-12-08T21:57:03.000+08:00
diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs
@@ -807,7 +807,7 @@ fn visit_lazy_tts_opt_mut<T: MutVisitor>(vis: &mut T, lazy_tts: Option<&mut Lazy
     }
 }
 
-fn visit_lazy_tts<T: MutVisitor>(vis: &mut T, lazy_tts: &mut Option<LazyAttrTokenStream>) {
+pub fn visit_lazy_tts<T: MutVisitor>(vis: &mut T, lazy_tts: &mut Option<LazyAttrTokenStream>) {
     visit_lazy_tts_opt_mut(vis, lazy_tts.as_mut());
 }
 
diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs
@@ -100,7 +100,7 @@ macro_rules! ast_fragments {
                 match self {
                     $($(AstFragment::$Kind(ast) => ast.extend(placeholders.iter().flat_map(|id| {
                         ${ignore($flat_map_ast_elt)}
-                        placeholder(AstFragmentKind::$Kind, *id, None).$make_ast()
+                        placeholder(false, AstFragmentKind::$Kind, *id, None).$make_ast()
                     })),)?)*
                     _ => panic!("unexpected AST fragment kind")
                 }
@@ -1783,17 +1783,26 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
         }
     }
 
-    fn collect(&mut self, fragment_kind: AstFragmentKind, kind: InvocationKind) -> AstFragment {
+    fn collect(
+        &mut self,
+        fragment_kind: AstFragmentKind,
+        invoke_kind: InvocationKind,
+    ) -> AstFragment {
         let expn_id = LocalExpnId::fresh_empty();
-        if matches!(kind, InvocationKind::GlobDelegation { .. }) {
+        if matches!(invoke_kind, InvocationKind::GlobDelegation { .. }) {
             // In resolver we need to know which invocation ids are delegations early,
             // before their `ExpnData` is filled.
             self.cx.resolver.register_glob_delegation(expn_id);
         }
-        let vis = kind.placeholder_visibility();
+        let vis = invoke_kind.placeholder_visibility();
+        let is_bang_brace = if let InvocationKind::Bang { mac, .. } = &invoke_kind {
+            mac.args.delim == Delimiter::Brace
+        } else {
+            false
+        };
         self.invocations.push((
             Invocation {
-                kind,
+                kind: invoke_kind,
                 fragment_kind,
                 expansion_data: ExpansionData {
                     id: expn_id,
@@ -1803,7 +1812,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
             },
             None,
         ));
-        placeholder(fragment_kind, NodeId::placeholder_from_expn_id(expn_id), vis)
+        placeholder(is_bang_brace, fragment_kind, NodeId::placeholder_from_expn_id(expn_id), vis)
     }
 
     fn collect_bang(&mut self, mac: P<ast::MacCall>, kind: AstFragmentKind) -> AstFragment {
diff --git a/compiler/rustc_expand/src/placeholders.rs b/compiler/rustc_expand/src/placeholders.rs
@@ -1,3 +1,5 @@
+use std::ops::DerefMut;
+
 use rustc_ast::mut_visit::*;
 use rustc_ast::ptr::P;
 use rustc_ast::token::Delimiter;
@@ -12,20 +14,21 @@ use thin_vec::ThinVec;
 use crate::expand::{AstFragment, AstFragmentKind};
 
 pub(crate) fn placeholder(
-    kind: AstFragmentKind,
+    is_bang_brace: bool,
+    fragment_kind: AstFragmentKind,
     id: ast::NodeId,
     vis: Option<ast::Visibility>,
 ) -> AstFragment {
-    fn mac_placeholder() -> P<ast::MacCall> {
+    let mac_placeholder = || -> P<ast::MacCall> {
         P(ast::MacCall {
             path: ast::Path { span: DUMMY_SP, segments: ThinVec::new(), tokens: None },
             args: P(ast::DelimArgs {
                 dspan: ast::tokenstream::DelimSpan::dummy(),
-                delim: Delimiter::Parenthesis,
+                delim: if is_bang_brace { Delimiter::Brace } else { Delimiter::Parenthesis },
                 tokens: ast::tokenstream::TokenStream::new(Vec::new()),
             }),
         })
-    }
+    };
 
     let ident = Ident::empty();
     let attrs = ast::AttrVec::new();
@@ -49,7 +52,7 @@ pub(crate) fn placeholder(
     let pat =
         || P(ast::Pat { id, kind: ast::PatKind::MacCall(mac_placeholder()), span, tokens: None });
 
-    match kind {
+    match fragment_kind {
         AstFragmentKind::Crate => AstFragment::Crate(ast::Crate {
             attrs: Default::default(),
             items: Default::default(),
@@ -194,6 +197,7 @@ pub(crate) fn placeholder(
 #[derive(Default)]
 pub(crate) struct PlaceholderExpander {
     expanded_fragments: FxHashMap<ast::NodeId, AstFragment>,
+    expr_under_let_init_else: bool,
 }
 
 impl PlaceholderExpander {
@@ -301,8 +305,48 @@ impl MutVisitor for PlaceholderExpander {
         }
     }
 
+    fn visit_local(&mut self, local: &mut P<ast::Local>) {
+        let ast::Local { id, pat, ty, kind, span, colon_sp, attrs, tokens } = local.deref_mut();
+        self.visit_id(id);
+        for attr in attrs.iter_mut() {
+            self.visit_attribute(attr);
+        }
+        self.visit_pat(pat);
+        if let Some(ty) = ty {
+            self.visit_ty(ty);
+        }
+        match kind {
+            ast::LocalKind::Decl => {}
+            ast::LocalKind::Init(init) => {
+                self.visit_expr(init);
+            }
+            ast::LocalKind::InitElse(init, els) => {
+                self.expr_under_let_init_else = true;
+                self.visit_expr(init);
+                self.expr_under_let_init_else = false;
+                self.visit_block(els);
+            }
+        }
+        visit_lazy_tts(self, tokens);
+        if let Some(sp) = colon_sp {
+            self.visit_span(sp);
+        }
+        self.visit_span(span);
+    }
+
     fn visit_expr(&mut self, expr: &mut P<ast::Expr>) {
-        match expr.kind {
+        tracing::debug!("visit: {:?}", expr);
+        match &mut expr.kind {
+            ast::ExprKind::Paren(paren) => {
+                if self.expr_under_let_init_else
+                    && let ast::ExprKind::MacCall(mac) = &paren.kind
+                    && matches!(mac.args.delim, Delimiter::Brace)
+                {
+                    *expr = self.remove(paren.id).make_expr();
+                } else {
+                    walk_expr(self, expr);
+                }
+            }
             ast::ExprKind::MacCall(_) => *expr = self.remove(expr.id).make_expr(),
             _ => walk_expr(self, expr),
         }
diff --git a/tests/ui/lint/unused-parens-for-macro-call-with-brace.fixed b/tests/ui/lint/unused-parens-for-macro-call-with-brace.fixed
@@ -0,0 +1,22 @@
+//@ run-rustfix
+
+#![deny(unused_parens)]
+
+fn main() {
+    macro_rules! x {
+        () => { None::<i32> };
+    }
+
+    let Some(_) = (x!{}) else { return }; // no error
+    let Some(_) = (x!{}) else { return };
+    //~^ ERROR: unnecessary parentheses around pattern
+    let _ = x!{};
+    let _ = x!{};
+    //~^ ERROR: unnecessary parentheses around assigned value
+    if let Some(_) = x!{} {};
+    if let Some(_) = x!{} {};
+    //~^ ERROR: unnecessary parentheses around `let` scrutinee expression
+    while let Some(_) = x!{} {};
+    while let Some(_) = x!{} {};
+    //~^ ERROR: unnecessary parentheses around `let` scrutinee expression
+}
diff --git a/tests/ui/lint/unused-parens-for-macro-call-with-brace.rs b/tests/ui/lint/unused-parens-for-macro-call-with-brace.rs
@@ -0,0 +1,22 @@
+//@ run-rustfix
+
+#![deny(unused_parens)]
+
+fn main() {
+    macro_rules! x {
+        () => { None::<i32> };
+    }
+
+    let Some(_) = (x!{}) else { return }; // no error
+    let Some((_)) = (x!{}) else { return };
+    //~^ ERROR: unnecessary parentheses around pattern
+    let _ = x!{};
+    let _ = (x!{});
+    //~^ ERROR: unnecessary parentheses around assigned value
+    if let Some(_) = x!{} {};
+    if let Some(_) = (x!{}) {};
+    //~^ ERROR: unnecessary parentheses around `let` scrutinee expression
+    while let Some(_) = x!{} {};
+    while let Some(_) = (x!{}) {};
+    //~^ ERROR: unnecessary parentheses around `let` scrutinee expression
+}
diff --git a/tests/ui/lint/unused-parens-for-macro-call-with-brace.stderr b/tests/ui/lint/unused-parens-for-macro-call-with-brace.stderr
@@ -0,0 +1,55 @@
+error: unnecessary parentheses around pattern
+  --> $DIR/unused-parens-for-macro-call-with-brace.rs:11:14
+   |
+LL |     let Some((_)) = (x!{}) else { return };
+   |              ^ ^
+   |
+note: the lint level is defined here
+  --> $DIR/unused-parens-for-macro-call-with-brace.rs:3:9
+   |
+LL | #![deny(unused_parens)]
+   |         ^^^^^^^^^^^^^
+help: remove these parentheses
+   |
+LL -     let Some((_)) = (x!{}) else { return };
+LL +     let Some(_) = (x!{}) else { return };
+   |
+
+error: unnecessary parentheses around assigned value
+  --> $DIR/unused-parens-for-macro-call-with-brace.rs:14:13
+   |
+LL |     let _ = (x!{});
+   |             ^    ^
+   |
+help: remove these parentheses
+   |
+LL -     let _ = (x!{});
+LL +     let _ = x!{};
+   |
+
+error: unnecessary parentheses around `let` scrutinee expression
+  --> $DIR/unused-parens-for-macro-call-with-brace.rs:17:22
+   |
+LL |     if let Some(_) = (x!{}) {};
+   |                      ^    ^
+   |
+help: remove these parentheses
+   |
+LL -     if let Some(_) = (x!{}) {};
+LL +     if let Some(_) = x!{} {};
+   |
+
+error: unnecessary parentheses around `let` scrutinee expression
+  --> $DIR/unused-parens-for-macro-call-with-brace.rs:20:25
+   |
+LL |     while let Some(_) = (x!{}) {};
+   |                         ^    ^
+   |
+help: remove these parentheses
+   |
+LL -     while let Some(_) = (x!{}) {};
+LL +     while let Some(_) = x!{} {};
+   |
+
+error: aborting due to 4 previous errors
+

Original file line number	Diff line number	Diff line change
`@@ -807,7 +807,7 @@ fn visit_lazy_tts_opt_mut<T: MutVisitor>(vis: &mut T, lazy_tts: Option<&mut Lazy`
`807`	`807`	`}`
`808`	`808`	`}`
`809`	`809`
`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>) {`
`811`	`811`	`visit_lazy_tts_opt_mut(vis, lazy_tts.as_mut());`
`812`	`812`	`}`
`813`	`813`