Skip to content
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: 2 additions & 2 deletions compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,12 +285,12 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {

#[inline(always)]
pub fn frame(&self) -> &Frame<'tcx, M::Provenance, M::FrameExtra> {
self.stack().last().expect("no call frames exist")
self.stack().last().expect("a call frame should exist")
}

#[inline(always)]
pub fn frame_mut(&mut self) -> &mut Frame<'tcx, M::Provenance, M::FrameExtra> {
self.stack_mut().last_mut().expect("no call frames exist")
self.stack_mut().last_mut().expect("a call frame should exist")
}

#[inline(always)]
Expand Down
22 changes: 17 additions & 5 deletions compiler/rustc_lint/src/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,14 @@ impl UnusedParens {
}
}

/// A pattern that a `macro_rules!` metavariable brings in from the call site keeps the call
/// site's syntax context, while the pattern's surroundings keep the macro's. Parentheses in
/// such a pattern can be required by the matcher even when they are redundant in the
/// expansion, so removing them would break the macro call. (Issue #86959)
fn pat_is_macro_arg(pat_span: Span, parent_span: Span) -> bool {
!pat_span.eq_ctxt(parent_span)
}

fn cast_followed_by_lt(&self, expr: &ast::Expr) -> Option<ast::NodeId> {
if let ExprKind::Binary(op, lhs, _rhs) = &expr.kind
&& (op.node == ast::BinOpKind::Lt || op.node == ast::BinOpKind::Shl)
Expand Down Expand Up @@ -811,7 +819,8 @@ impl EarlyLintPass for UnusedParens {

match e.kind {
ExprKind::Let(ref pat, _, _, _) | ExprKind::ForLoop(ForLoop { ref pat, .. }) => {
self.check_unused_parens_pat(cx, pat, false, false, (true, true));
let avoid_or = Self::pat_is_macro_arg(pat.span, e.span);
self.check_unused_parens_pat(cx, pat, avoid_or, false, (true, true));
}
// We ignore parens in cases like `if (((let Some(0) = Some(1))))` because we already
// handle a hard error for them during AST lowering in `lower_expr_mut`, but we still
Expand Down Expand Up @@ -890,13 +899,15 @@ impl EarlyLintPass for UnusedParens {
| Err(_) => {}
// These are list-like patterns; parens can always be removed.
TupleStruct(_, _, ps) | Tuple(ps) | Slice(ps) | Or(ps) => {
for p in ps {
self.check_unused_parens_pat(cx, p, false, false, keep_space);
for sub in ps {
let avoid_or = Self::pat_is_macro_arg(sub.span, p.span);
self.check_unused_parens_pat(cx, sub, avoid_or, false, keep_space);
}
}
Struct(_, _, fps, _) => {
for f in fps {
self.check_unused_parens_pat(cx, &f.pat, false, false, keep_space);
let avoid_or = Self::pat_is_macro_arg(f.pat.span, p.span);
self.check_unused_parens_pat(cx, &f.pat, avoid_or, false, keep_space);
}
}
// Avoid linting on `i @ (p0 | .. | pn)`, #64106.
Expand Down Expand Up @@ -938,7 +949,8 @@ impl EarlyLintPass for UnusedParens {
}

fn check_arm(&mut self, cx: &EarlyContext<'_>, arm: &ast::Arm) {
self.check_unused_parens_pat(cx, &arm.pat, false, false, (false, false));
let avoid_or = Self::pat_is_macro_arg(arm.pat.span, arm.span);
self.check_unused_parens_pat(cx, &arm.pat, avoid_or, false, (false, false));
}

fn check_ty(&mut self, cx: &EarlyContext<'_>, ty: &ast::Ty) {
Expand Down
13 changes: 11 additions & 2 deletions library/alloc/src/collections/btree/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,17 @@ pub struct BTreeMap<
#[stable(feature = "btree_drop", since = "1.7.0")]
unsafe impl<#[may_dangle] K, #[may_dangle] V, A: AllocatorClone> Drop for BTreeMap<K, V, A> {
fn drop(&mut self) {
// ignore-tidy-undocumented-unsafe
drop(unsafe { ptr::read(self) }.into_iter())
// Skip `into_iter` for an empty map: `dying_next` is too costly to inline, so the
// empty drop isn't optimised away (see #161375).
if self.root.is_some() {
// SAFETY: `self` is not used after this and none of its fields are dropped again:
// `alloc` is `ManuallyDrop` and `root` has no drop glue.
drop(unsafe { ptr::read(self) }.into_iter())
} else {
// SAFETY: With no root there are no nodes to free, so only the allocator needs
// dropping. `self` is not used after this, and `alloc` is dropped only here.
unsafe { ManuallyDrop::drop(&mut self.alloc) }
}
}
}

Expand Down
15 changes: 15 additions & 0 deletions tests/codegen-llvm/btree-empty-drop.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Regression test for https://github.com/rust-lang/rust/issues/161375
// Checking that an empty BTree's drop is optimized away.
//@ compile-flags: -Copt-level=3

#![crate_type = "lib"]

use std::collections::BTreeMap;

// CHECK-LABEL: @drop_btree(
// CHECK-NEXT: {{.*}}:
// CHECK-NEXT: ret void
#[no_mangle]
pub fn drop_btree() {
let _ = BTreeMap::<(), ()>::new();
}
42 changes: 42 additions & 0 deletions tests/ui/lint/unused-parens-macro-pat-issue-86959.e2018.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
error: unnecessary parentheses around pattern
--> $DIR/unused-parens-macro-pat-issue-86959.rs:76:16
|
LL | match_pat!(('a'));
| ^ ^
|
note: the lint level is defined here
--> $DIR/unused-parens-macro-pat-issue-86959.rs:9:9
|
LL | #![deny(unused_parens)]
| ^^^^^^^^^^^^^
help: remove these parentheses
|
LL - match_pat!(('a'));
LL + match_pat!('a');
|

error: unnecessary parentheses around pattern
--> $DIR/unused-parens-macro-pat-issue-86959.rs:54:13
|
LL | macro_rules! body_or_pat {
...
LL | ('a' | 'A') => {}
| ^^^^^^^^^^^
...
LL | body_or_pat!();
| -------------- in this macro invocation

error: unnecessary parentheses around pattern
--> $DIR/unused-parens-macro-pat-issue-86959.rs:80:9
|
LL | let (_x) = 1;
| ^ ^
|
help: remove these parentheses
|
LL - let (_x) = 1;
LL + let _x = 1;
|

error: aborting due to 3 previous errors

42 changes: 42 additions & 0 deletions tests/ui/lint/unused-parens-macro-pat-issue-86959.e2021.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
error: unnecessary parentheses around pattern
--> $DIR/unused-parens-macro-pat-issue-86959.rs:76:16
|
LL | match_pat!(('a'));
| ^ ^
|
note: the lint level is defined here
--> $DIR/unused-parens-macro-pat-issue-86959.rs:9:9
|
LL | #![deny(unused_parens)]
| ^^^^^^^^^^^^^
help: remove these parentheses
|
LL - match_pat!(('a'));
LL + match_pat!('a');
|

error: unnecessary parentheses around pattern
--> $DIR/unused-parens-macro-pat-issue-86959.rs:54:13
|
LL | macro_rules! body_or_pat {
...
LL | ('a' | 'A') => {}
| ^^^^^^^^^^^
...
LL | body_or_pat!();
| -------------- in this macro invocation

error: unnecessary parentheses around pattern
--> $DIR/unused-parens-macro-pat-issue-86959.rs:80:9
|
LL | let (_x) = 1;
| ^ ^
|
help: remove these parentheses
|
LL - let (_x) = 1;
LL + let _x = 1;
|

error: aborting due to 3 previous errors

81 changes: 81 additions & 0 deletions tests/ui/lint/unused-parens-macro-pat-issue-86959.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
//@ revisions: e2018 e2021
//@[e2018] edition: 2018
//@[e2021] edition: 2021

// Parentheses around a pattern handed to a `macro_rules!` metavariable can be required by the
// matcher even though they are redundant in the expanded pattern, so removing them breaks the
// macro call. rust-lang/rust#86959

#![deny(unused_parens)]

macro_rules! match_pat {
($p:pat) => {
match 'a' {
$p => {}
_ => {}
}
};
}

macro_rules! match_pat_param {
($p:pat_param) => {
match 'a' {
$p => {}
_ => {}
}
};
}

macro_rules! if_let_pat {
($p:pat) => {
if let $p = 'a' {}
};
}

macro_rules! nested_pat {
($p:pat) => {
match Some(1) {
Some($p) => {}
_ => {}
}
};
}

// `body_or_pat!()` breaks the context leg. its a `PatKind::Or`, but the parens and the arm
// both come from the macro body so they share one `SyntaxContext`, `eq_ctxt` is true and
// `avoid_or` is false. these are the macro authors own parens, no metavariable involved, so
// removing them is safe. this is the case that matters most: `from_expansion()` is true here
// and `!eq_ctxt` is false, so if the gate were ever keyed on "are we inside a macro" instead
// of "did this cross a context boundary" it would be wrongly silenced. nothing else in the
// file detects that.
macro_rules! body_or_pat {
() => {
match 'a' {
('a' | 'A') => {} //~ ERROR unnecessary parentheses around pattern
_ => {}
}
};
}

fn main() {
// `$p:pat` only accepts a top-level `|` from edition 2021 onwards.
match_pat!(('a' | 'A'));
// `$p:pat_param` never accepts a top-level `|`, in any edition.
match_pat_param!(('a' | 'A'));
if_let_pat!(('a' | 'A'));
nested_pat!((1 | 2));
// A leading `..=` is rejected by both fragment specifiers. rust-lang/rust#120737
match_pat!((..='a' | 'z'));

// `match_pat!(('a'))` breaks the or leg. `avoid_or` is true, since the pattern arrives
// through `$p:pat` and its `SyntaxContext` differs from the arms. but the inner pattern is
// `PatKind::Expr`, not `PatKind::Or`, so `unused.rs:676` never matches and the lint fires.
// the suggestion rewrites the call site to `match_pat!('a')`, which is right — `$p:pat`
// takes a bare literal. this guards against widening the gate to all patterns, or moving
// the `avoid_or` check above the `match inner.kind`.
match_pat!(('a')); //~ ERROR unnecessary parentheses around pattern
body_or_pat!();
// `let (_x) = 1;` is the no-macro baseline for the `check_expr` call site, one of the four
// that went from a hardcoded `false` to a computed value.
let (_x) = 1; //~ ERROR unnecessary parentheses around pattern
}
Loading