From e0a08351a23d06903badd6f79d0971d2adf1b645 Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Fri, 27 Mar 2020 21:53:07 +0100 Subject: [PATCH 01/12] add visit_anon_const to EarlyContextAndPass --- src/librustc_lint/early.rs | 5 +++++ src/librustc_lint/passes.rs | 1 + 2 files changed, 6 insertions(+) diff --git a/src/librustc_lint/early.rs b/src/librustc_lint/early.rs index 34da29c974777..018e9da243c70 100644 --- a/src/librustc_lint/early.rs +++ b/src/librustc_lint/early.rs @@ -104,6 +104,11 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T> run_early_pass!(self, check_pat_post, p); } + fn visit_anon_const(&mut self, c: &'a ast::AnonConst) { + run_early_pass!(self, check_anon_const, c); + ast_visit::walk_anon_const(self, c); + } + fn visit_expr(&mut self, e: &'a ast::Expr) { self.with_lint_attrs(e.id, &e.attrs, |cx| { run_early_pass!(cx, check_expr, e); diff --git a/src/librustc_lint/passes.rs b/src/librustc_lint/passes.rs index ace154714458e..c9e12afedbbab 100644 --- a/src/librustc_lint/passes.rs +++ b/src/librustc_lint/passes.rs @@ -170,6 +170,7 @@ macro_rules! early_lint_methods { fn check_stmt(a: &ast::Stmt); fn check_arm(a: &ast::Arm); fn check_pat(a: &ast::Pat); + fn check_anon_const(a: &ast::AnonConst); fn check_pat_post(a: &ast::Pat); fn check_expr(a: &ast::Expr); fn check_expr_post(a: &ast::Expr); From b3d744c9f51b6bb42c7cebeb8f7fcefac8778d1e Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Fri, 27 Mar 2020 21:54:52 +0100 Subject: [PATCH 02/12] add `unused_braces`, lint anon_const --- src/librustc_lint/lib.rs | 2 + src/librustc_lint/unused.rs | 493 ++++++++++++++++++++++++++---------- 2 files changed, 366 insertions(+), 129 deletions(-) diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 825ac04bc0920..1b92e77558505 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -104,6 +104,7 @@ macro_rules! early_lint_passes { $args, [ UnusedParens: UnusedParens, + UnusedBraces: UnusedBraces, UnusedImportBraces: UnusedImportBraces, UnsafeCode: UnsafeCode, AnonymousParameters: AnonymousParameters, @@ -275,6 +276,7 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) { UNUSED_FEATURES, UNUSED_LABELS, UNUSED_PARENS, + UNUSED_BRACES, REDUNDANT_SEMICOLONS ); diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index b5826d6a5efa6..67e86c480a30f 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -1,7 +1,9 @@ +use crate::Lint; use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext}; use rustc::ty::adjustment; use rustc::ty::{self, Ty}; use rustc_ast::ast; +use rustc_ast::ast::{ExprKind, StmtKind}; use rustc_ast::attr; use rustc_ast::util::parser; use rustc_ast_pretty::pprust; @@ -318,16 +320,58 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedAttributes { } } -declare_lint! { - pub(super) UNUSED_PARENS, - Warn, - "`if`, `match`, `while` and `return` do not need parentheses" +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +enum UnusedDelimsCtx { + FunctionArg, + MethodArg, + AssignedValue, + IfCond, + WhileCond, + ForHeadExpr, + MatchHeadExpr, + ReturnValue, + BlockRetValue, + LetHeadExpr, + ArrayLenExpr, + AnonConst, } -declare_lint_pass!(UnusedParens => [UNUSED_PARENS]); +impl From for &'static str { + fn from(ctx: UnusedDelimsCtx) -> &'static str { + match ctx { + UnusedDelimsCtx::FunctionArg => "function argument", + UnusedDelimsCtx::MethodArg => "method argument", + UnusedDelimsCtx::AssignedValue => "assigned value", + UnusedDelimsCtx::IfCond => "`if` condition", + UnusedDelimsCtx::WhileCond => "`while` condition", + UnusedDelimsCtx::ForHeadExpr => "`for` head expression", + UnusedDelimsCtx::MatchHeadExpr => "`match` head expression", + UnusedDelimsCtx::ReturnValue => "`return` value", + UnusedDelimsCtx::BlockRetValue => "block return value", + UnusedDelimsCtx::LetHeadExpr => "`let` head expression", + UnusedDelimsCtx::ArrayLenExpr | UnusedDelimsCtx::AnonConst => "const expression", + } + } +} -impl UnusedParens { - fn is_expr_parens_necessary(inner: &ast::Expr, followed_by_block: bool) -> bool { +/// Used by both `UnusedParens` and `UnusedBraces` to prevent code duplication. +trait UnusedDelimLint { + const DELIM_STR: &'static str; + + // this cannot be a constant is it refers to a static. + fn lint(&self) -> &'static Lint; + + fn check_unused_delims_expr( + &self, + cx: &EarlyContext<'_>, + value: &ast::Expr, + ctx: UnusedDelimsCtx, + followed_by_block: bool, + left_pos: Option, + right_pos: Option, + ); + + fn is_expr_delims_necessary(inner: &ast::Expr, followed_by_block: bool) -> bool { followed_by_block && match inner.kind { ast::ExprKind::Ret(_) | ast::ExprKind::Break(..) => true, @@ -335,98 +379,42 @@ impl UnusedParens { } } - fn check_unused_parens_expr( + fn emit_unused_delims_expr( &self, cx: &EarlyContext<'_>, value: &ast::Expr, - msg: &str, - followed_by_block: bool, + ctx: UnusedDelimsCtx, left_pos: Option, right_pos: Option, ) { - match value.kind { - ast::ExprKind::Paren(ref inner) => { - if !Self::is_expr_parens_necessary(inner, followed_by_block) - && value.attrs.is_empty() - && !value.span.from_expansion() - { - let expr_text = - if let Ok(snippet) = cx.sess().source_map().span_to_snippet(value.span) { - snippet - } else { - pprust::expr_to_string(value) - }; - let keep_space = ( - left_pos.map(|s| s >= value.span.lo()).unwrap_or(false), - right_pos.map(|s| s <= value.span.hi()).unwrap_or(false), - ); - Self::remove_outer_parens(cx, value.span, &expr_text, msg, keep_space); - } - } - ast::ExprKind::Let(_, ref expr) => { - // FIXME(#60336): Properly handle `let true = (false && true)` - // actually needing the parenthesis. - self.check_unused_parens_expr( - cx, - expr, - "`let` head expression", - followed_by_block, - None, - None, - ); - } - _ => {} - } + let expr_text = if let Ok(snippet) = cx.sess().source_map().span_to_snippet(value.span) { + snippet + } else { + pprust::expr_to_string(value) + }; + let keep_space = ( + left_pos.map(|s| s >= value.span.lo()).unwrap_or(false), + right_pos.map(|s| s <= value.span.hi()).unwrap_or(false), + ); + self.emit_unused_delims(cx, value.span, &expr_text, ctx.into(), keep_space); } - fn check_unused_parens_pat( + /// emits a lint + fn emit_unused_delims( &self, - cx: &EarlyContext<'_>, - value: &ast::Pat, - avoid_or: bool, - avoid_mut: bool, - ) { - use ast::{BindingMode, Mutability, PatKind}; - - if let PatKind::Paren(inner) = &value.kind { - match inner.kind { - // The lint visitor will visit each subpattern of `p`. We do not want to lint - // any range pattern no matter where it occurs in the pattern. For something like - // `&(a..=b)`, there is a recursive `check_pat` on `a` and `b`, but we will assume - // that if there are unnecessary parens they serve a purpose of readability. - PatKind::Range(..) => return, - // Avoid `p0 | .. | pn` if we should. - PatKind::Or(..) if avoid_or => return, - // Avoid `mut x` and `mut x @ p` if we should: - PatKind::Ident(BindingMode::ByValue(Mutability::Mut), ..) if avoid_mut => return, - // Otherwise proceed with linting. - _ => {} - } - - let pattern_text = - if let Ok(snippet) = cx.sess().source_map().span_to_snippet(value.span) { - snippet - } else { - pprust::pat_to_string(value) - }; - Self::remove_outer_parens(cx, value.span, &pattern_text, "pattern", (false, false)); - } - } - - fn remove_outer_parens( cx: &EarlyContext<'_>, span: Span, pattern: &str, msg: &str, keep_space: (bool, bool), ) { - cx.struct_span_lint(UNUSED_PARENS, span, |lint| { - let span_msg = format!("unnecessary parentheses around {}", msg); + cx.struct_span_lint(self.lint(), span, |lint| { + let span_msg = format!("unnecessary {} around {}", Self::DELIM_STR, msg); let mut err = lint.build(&span_msg); let mut ate_left_paren = false; let mut ate_right_paren = false; let parens_removed = pattern.trim_matches(|c| match c { - '(' => { + '(' | '{' => { if ate_left_paren { false } else { @@ -434,7 +422,7 @@ impl UnusedParens { true } } - ')' => { + ')' | '}' => { if ate_right_paren { false } else { @@ -460,61 +448,51 @@ impl UnusedParens { replace }; - err.span_suggestion_short( - span, - "remove these parentheses", - replace, - Applicability::MachineApplicable, - ); + let suggestion = format!("remove these {}", Self::DELIM_STR); + + err.span_suggestion_short(span, &suggestion, replace, Applicability::MachineApplicable); err.emit(); }); } -} -impl EarlyLintPass for UnusedParens { fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) { use rustc_ast::ast::ExprKind::*; - let (value, msg, followed_by_block, left_pos, right_pos) = match e.kind { - Let(ref pat, ..) => { - self.check_unused_parens_pat(cx, pat, false, false); - return; - } - + let (value, ctx, followed_by_block, left_pos, right_pos) = match e.kind { If(ref cond, ref block, ..) => { let left = e.span.lo() + rustc_span::BytePos(2); let right = block.span.lo(); - (cond, "`if` condition", true, Some(left), Some(right)) + (cond, UnusedDelimsCtx::IfCond, true, Some(left), Some(right)) } While(ref cond, ref block, ..) => { let left = e.span.lo() + rustc_span::BytePos(5); let right = block.span.lo(); - (cond, "`while` condition", true, Some(left), Some(right)) + (cond, UnusedDelimsCtx::WhileCond, true, Some(left), Some(right)) } - ForLoop(ref pat, ref cond, ref block, ..) => { - self.check_unused_parens_pat(cx, pat, false, false); - (cond, "`for` head expression", true, None, Some(block.span.lo())) + ForLoop(_, ref cond, ref block, ..) => { + (cond, UnusedDelimsCtx::ForHeadExpr, true, None, Some(block.span.lo())) } Match(ref head, _) => { let left = e.span.lo() + rustc_span::BytePos(5); - (head, "`match` head expression", true, Some(left), None) + (head, UnusedDelimsCtx::MatchHeadExpr, true, Some(left), None) } Ret(Some(ref value)) => { let left = e.span.lo() + rustc_span::BytePos(3); - (value, "`return` value", false, Some(left), None) + (value, UnusedDelimsCtx::ReturnValue, false, Some(left), None) } - Assign(_, ref value, _) => (value, "assigned value", false, None, None), - AssignOp(.., ref value) => (value, "assigned value", false, None, None), + Assign(_, ref value, _) | AssignOp(.., ref value) => { + (value, UnusedDelimsCtx::AssignedValue, false, None, None) + } // either function/method call, or something this lint doesn't care about ref call_or_other => { - let (args_to_check, call_kind) = match *call_or_other { - Call(_, ref args) => (&args[..], "function"), - // first "argument" is self (which sometimes needs parens) - MethodCall(_, ref args) => (&args[1..], "method"), + let (args_to_check, ctx) = match *call_or_other { + Call(_, ref args) => (&args[..], UnusedDelimsCtx::FunctionArg), + // first "argument" is self (which sometimes needs delims) + MethodCall(_, ref args) => (&args[1..], UnusedDelimsCtx::MethodArg), // actual catch-all arm _ => { return; @@ -527,14 +505,152 @@ impl EarlyLintPass for UnusedParens { if e.span.ctxt().outer_expn_data().call_site.from_expansion() { return; } - let msg = format!("{} argument", call_kind); for arg in args_to_check { - self.check_unused_parens_expr(cx, arg, &msg, false, None, None); + self.check_unused_delims_expr(cx, arg, ctx, false, None, None); } return; } }; - self.check_unused_parens_expr(cx, &value, msg, followed_by_block, left_pos, right_pos); + self.check_unused_delims_expr(cx, &value, ctx, followed_by_block, left_pos, right_pos); + } + + fn check_stmt(&mut self, cx: &EarlyContext<'_>, s: &ast::Stmt) { + match s.kind { + StmtKind::Local(ref local) => { + if let Some(ref value) = local.init { + self.check_unused_delims_expr( + cx, + &value, + UnusedDelimsCtx::AssignedValue, + false, + None, + None, + ); + } + } + StmtKind::Expr(ref expr) => { + self.check_unused_delims_expr( + cx, + &expr, + UnusedDelimsCtx::BlockRetValue, + false, + None, + None, + ); + } + _ => {} + } + } + + fn check_item(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) { + use ast::ItemKind::*; + + if let Const(.., Some(expr)) | Static(.., Some(expr)) = &item.kind { + self.check_unused_delims_expr( + cx, + expr, + UnusedDelimsCtx::AssignedValue, + false, + None, + None, + ); + } + } +} + +declare_lint! { + pub(super) UNUSED_PARENS, + Warn, + "`if`, `match`, `while` and `return` do not need parentheses" +} + +declare_lint_pass!(UnusedParens => [UNUSED_PARENS]); + +impl UnusedDelimLint for UnusedParens { + const DELIM_STR: &'static str = "parentheses"; + + fn lint(&self) -> &'static Lint { + UNUSED_PARENS + } + + fn check_unused_delims_expr( + &self, + cx: &EarlyContext<'_>, + value: &ast::Expr, + ctx: UnusedDelimsCtx, + followed_by_block: bool, + left_pos: Option, + right_pos: Option, + ) { + match value.kind { + ast::ExprKind::Paren(ref inner) => { + if !Self::is_expr_delims_necessary(inner, followed_by_block) + && value.attrs.is_empty() + && !value.span.from_expansion() + { + self.emit_unused_delims_expr(cx, value, ctx, left_pos, right_pos) + } + } + ast::ExprKind::Let(_, ref expr) => { + // FIXME(#60336): Properly handle `let true = (false && true)` + // actually needing the parenthesis. + self.check_unused_delims_expr( + cx, + expr, + UnusedDelimsCtx::LetHeadExpr, + followed_by_block, + None, + None, + ); + } + _ => {} + } + } +} + +impl UnusedParens { + fn check_unused_parens_pat( + &self, + cx: &EarlyContext<'_>, + value: &ast::Pat, + avoid_or: bool, + avoid_mut: bool, + ) { + use ast::{BindingMode, Mutability, PatKind}; + + if let PatKind::Paren(inner) = &value.kind { + match inner.kind { + // The lint visitor will visit each subpattern of `p`. We do not want to lint + // any range pattern no matter where it occurs in the pattern. For something like + // `&(a..=b)`, there is a recursive `check_pat` on `a` and `b`, but we will assume + // that if there are unnecessary parens they serve a purpose of readability. + PatKind::Range(..) => return, + // Avoid `p0 | .. | pn` if we should. + PatKind::Or(..) if avoid_or => return, + // Avoid `mut x` and `mut x @ p` if we should: + PatKind::Ident(BindingMode::ByValue(Mutability::Mut), ..) if avoid_mut => return, + // Otherwise proceed with linting. + _ => {} + } + + let pattern_text = + if let Ok(snippet) = cx.sess().source_map().span_to_snippet(value.span) { + snippet + } else { + pprust::pat_to_string(value) + }; + self.emit_unused_delims(cx, value.span, &pattern_text, "pattern", (false, false)); + } + } +} + +impl EarlyLintPass for UnusedParens { + fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) { + if let ExprKind::Let(ref pat, ..) | ExprKind::ForLoop(ref pat, ..) = e.kind { + self.check_unused_parens_pat(cx, pat, false, false); + } + + ::check_expr(self, cx, e) } fn check_pat(&mut self, cx: &EarlyContext<'_>, p: &ast::Pat) { @@ -559,22 +675,16 @@ impl EarlyLintPass for UnusedParens { } } - fn check_stmt(&mut self, cx: &EarlyContext<'_>, s: &ast::Stmt) { - use ast::StmtKind::*; - - match s.kind { - Local(ref local) => { - self.check_unused_parens_pat(cx, &local.pat, false, false); + fn check_anon_const(&mut self, cx: &EarlyContext<'_>, c: &ast::AnonConst) { + self.check_unused_delims_expr(cx, &c.value, UnusedDelimsCtx::AnonConst, false, None, None); + } - if let Some(ref value) = local.init { - self.check_unused_parens_expr(cx, &value, "assigned value", false, None, None); - } - } - Expr(ref expr) => { - self.check_unused_parens_expr(cx, &expr, "block return value", false, None, None); - } - _ => {} + fn check_stmt(&mut self, cx: &EarlyContext<'_>, s: &ast::Stmt) { + if let StmtKind::Local(ref local) = s.kind { + self.check_unused_parens_pat(cx, &local.pat, false, false); } + + ::check_stmt(self, cx, s) } fn check_param(&mut self, cx: &EarlyContext<'_>, param: &ast::Param) { @@ -590,6 +700,16 @@ impl EarlyLintPass for UnusedParens { match &r.kind { &ast::TyKind::TraitObject(..) => {} &ast::TyKind::ImplTrait(_, ref bounds) if bounds.len() > 1 => {} + &ast::TyKind::Array(_, ref len) => { + self.check_unused_delims_expr( + cx, + &len.value, + UnusedDelimsCtx::ArrayLenExpr, + false, + None, + None, + ); + } _ => { let pattern_text = if let Ok(snippet) = cx.sess().source_map().span_to_snippet(ty.span) { @@ -598,19 +718,134 @@ impl EarlyLintPass for UnusedParens { pprust::ty_to_string(ty) }; - Self::remove_outer_parens(cx, ty.span, &pattern_text, "type", (false, false)); + self.emit_unused_delims(cx, ty.span, &pattern_text, "type", (false, false)); } } } } fn check_item(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) { - use ast::ItemKind::*; + ::check_item(self, cx, item) + } +} - if let Const(.., Some(expr)) | Static(.., Some(expr)) = &item.kind { - self.check_unused_parens_expr(cx, expr, "assigned value", false, None, None); +declare_lint! { + pub(super) UNUSED_BRACES, + Warn, + "suggests removing `{` and `}` in case they are not necessary" +} + +declare_lint_pass!(UnusedBraces => [UNUSED_BRACES]); + +impl UnusedDelimLint for UnusedBraces { + const DELIM_STR: &'static str = "braces"; + + fn lint(&self) -> &'static Lint { + UNUSED_BRACES + } + + fn check_unused_delims_expr( + &self, + cx: &EarlyContext<'_>, + value: &ast::Expr, + ctx: UnusedDelimsCtx, + followed_by_block: bool, + left_pos: Option, + right_pos: Option, + ) { + match value.kind { + ast::ExprKind::Block(ref inner, None) + if inner.rules == ast::BlockCheckMode::Default => + { + // emit a warning under the following conditions: + // + // - the block does not have a label + // - 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; let _: A<{ 2 + 3 }>;`) + // + // 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) + && (ctx != UnusedDelimsCtx::AnonConst + || matches!(expr.kind, ast::ExprKind::Lit(_))) + // array length expressions are checked during `check_anon_const` and `check_ty`, + // once as `ArrayLenExpr` and once as `AnonConst`. + // + // As we do not want to lint this twice, we do not emit an error for + // `ArrayLenExpr` if `AnonConst` would do the same. + && (ctx != UnusedDelimsCtx::ArrayLenExpr + || !matches!(expr.kind, ast::ExprKind::Lit(_))) + && !cx.sess().source_map().is_multiline(value.span) + && value.attrs.is_empty() + && !value.span.from_expansion() + { + self.emit_unused_delims_expr(cx, value, ctx, left_pos, right_pos) + } + } + } + } + ast::ExprKind::Let(_, ref expr) => { + // FIXME(#60336): Properly handle `let true = (false && true)` + // actually needing the parenthesis. + self.check_unused_delims_expr( + cx, + expr, + UnusedDelimsCtx::LetHeadExpr, + followed_by_block, + None, + None, + ); + } + _ => {} + } + } +} + +impl EarlyLintPass for UnusedBraces { + fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) { + ::check_expr(self, cx, e) + } + + fn check_anon_const(&mut self, cx: &EarlyContext<'_>, c: &ast::AnonConst) { + self.check_unused_delims_expr(cx, &c.value, UnusedDelimsCtx::AnonConst, false, None, None); + } + + fn check_stmt(&mut self, cx: &EarlyContext<'_>, s: &ast::Stmt) { + ::check_stmt(self, cx, s) + } + + fn check_ty(&mut self, cx: &EarlyContext<'_>, ty: &ast::Ty) { + if let &ast::TyKind::Paren(ref r) = &ty.kind { + if let ast::TyKind::Array(_, ref len) = r.kind { + self.check_unused_delims_expr( + cx, + &len.value, + UnusedDelimsCtx::ArrayLenExpr, + false, + None, + None, + ); + } } } + + fn check_item(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) { + ::check_item(self, cx, item) + } } declare_lint! { From a6cae3d5cfad59e16afa95926324bf8e93250019 Mon Sep 17 00:00:00 2001 From: Stein Somers Date: Sun, 2 Feb 2020 12:11:41 +0100 Subject: [PATCH 03/12] Add benchmarks of drain_filter-like behaviour --- src/liballoc/benches/btree/set.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/liballoc/benches/btree/set.rs b/src/liballoc/benches/btree/set.rs index d9e75ab7fa4ef..c4712e2132f78 100644 --- a/src/liballoc/benches/btree/set.rs +++ b/src/liballoc/benches/btree/set.rs @@ -62,6 +62,18 @@ pub fn clone_100_and_clear(b: &mut Bencher) { b.iter(|| src.clone().clear()) } +#[bench] +pub fn clone_100_and_drain_half(b: &mut Bencher) { + let src = pos(100); + b.iter(|| { + let mut set = src.clone(); + for i in set.iter().copied().filter(|i| i % 2 == 0).collect::>() { + set.remove(&i); + } + assert_eq!(set.len(), 100 / 2); + }) +} + #[bench] pub fn clone_100_and_into_iter(b: &mut Bencher) { let src = pos(100); @@ -115,6 +127,18 @@ pub fn clone_10k_and_clear(b: &mut Bencher) { b.iter(|| src.clone().clear()) } +#[bench] +pub fn clone_10k_and_drain_half(b: &mut Bencher) { + let src = pos(10_000); + b.iter(|| { + let mut set = src.clone(); + for i in set.iter().copied().filter(|i| i % 2 == 0).collect::>() { + set.remove(&i); + } + assert_eq!(set.len(), 10_000 / 2); + }) +} + #[bench] pub fn clone_10k_and_into_iter(b: &mut Bencher) { let src = pos(10_000); From 0405db3a341a094f421a762a15c14b392fec94f8 Mon Sep 17 00:00:00 2001 From: Stein Somers Date: Sat, 8 Feb 2020 11:52:14 +0100 Subject: [PATCH 04/12] BTreeMap/BTreeSet: implement and test drain_filter --- src/liballoc/benches/btree/set.rs | 20 +- src/liballoc/benches/lib.rs | 1 + src/liballoc/collections/btree/map.rs | 210 ++++++++++++++++++++- src/liballoc/collections/btree/set.rs | 101 +++++++++- src/liballoc/tests/btree/map.rs | 259 +++++++++++++++++++++++++- src/liballoc/tests/btree/set.rs | 81 ++++++++ src/liballoc/tests/lib.rs | 1 + 7 files changed, 653 insertions(+), 20 deletions(-) diff --git a/src/liballoc/benches/btree/set.rs b/src/liballoc/benches/btree/set.rs index c4712e2132f78..2518506b9b5f3 100644 --- a/src/liballoc/benches/btree/set.rs +++ b/src/liballoc/benches/btree/set.rs @@ -62,14 +62,18 @@ pub fn clone_100_and_clear(b: &mut Bencher) { b.iter(|| src.clone().clear()) } +#[bench] +pub fn clone_100_and_drain_all(b: &mut Bencher) { + let src = pos(100); + b.iter(|| src.clone().drain_filter(|_| true).count()) +} + #[bench] pub fn clone_100_and_drain_half(b: &mut Bencher) { let src = pos(100); b.iter(|| { let mut set = src.clone(); - for i in set.iter().copied().filter(|i| i % 2 == 0).collect::>() { - set.remove(&i); - } + assert_eq!(set.drain_filter(|i| i % 2 == 0).count(), 100 / 2); assert_eq!(set.len(), 100 / 2); }) } @@ -127,14 +131,18 @@ pub fn clone_10k_and_clear(b: &mut Bencher) { b.iter(|| src.clone().clear()) } +#[bench] +pub fn clone_10k_and_drain_all(b: &mut Bencher) { + let src = pos(10_000); + b.iter(|| src.clone().drain_filter(|_| true).count()) +} + #[bench] pub fn clone_10k_and_drain_half(b: &mut Bencher) { let src = pos(10_000); b.iter(|| { let mut set = src.clone(); - for i in set.iter().copied().filter(|i| i % 2 == 0).collect::>() { - set.remove(&i); - } + assert_eq!(set.drain_filter(|i| i % 2 == 0).count(), 10_000 / 2); assert_eq!(set.len(), 10_000 / 2); }) } diff --git a/src/liballoc/benches/lib.rs b/src/liballoc/benches/lib.rs index 951477a24c8ed..f31717d9fd517 100644 --- a/src/liballoc/benches/lib.rs +++ b/src/liballoc/benches/lib.rs @@ -1,3 +1,4 @@ +#![feature(btree_drain_filter)] #![feature(map_first_last)] #![feature(repr_simd)] #![feature(test)] diff --git a/src/liballoc/collections/btree/map.rs b/src/liballoc/collections/btree/map.rs index bde66c406af7f..bbeced1751d14 100644 --- a/src/liballoc/collections/btree/map.rs +++ b/src/liballoc/collections/btree/map.rs @@ -1256,6 +1256,48 @@ impl BTreeMap { right } + /// Creates an iterator which uses a closure to determine if an element should be removed. + /// + /// If the closure returns true, the element is removed from the map and yielded. + /// If the closure returns false, or panics, the element remains in the map and will not be + /// yielded. + /// + /// Note that `drain_filter` lets you mutate every value in the filter closure, regardless of + /// whether you choose to keep or remove it. + /// + /// If the iterator is only partially consumed or not consumed at all, each of the remaining + /// elements will still be subjected to the closure and removed and dropped if it returns true. + /// + /// It is unspecified how many more elements will be subjected to the closure + /// if a panic occurs in the closure, or a panic occurs while dropping an element, + /// or if the `DrainFilter` value is leaked. + /// + /// # Examples + /// + /// Splitting a map into even and odd keys, reusing the original map: + /// + /// ``` + /// #![feature(btree_drain_filter)] + /// use std::collections::BTreeMap; + /// + /// let mut map: BTreeMap = (0..8).map(|x| (x, x)).collect(); + /// let evens: BTreeMap<_, _> = map.drain_filter(|k, _v| k % 2 == 0).collect(); + /// let odds = map; + /// assert_eq!(evens.keys().copied().collect::>(), vec![0, 2, 4, 6]); + /// assert_eq!(odds.keys().copied().collect::>(), vec![1, 3, 5, 7]); + /// ``` + #[unstable(feature = "btree_drain_filter", issue = "70530")] + pub fn drain_filter(&mut self, pred: F) -> DrainFilter<'_, K, V, F> + where + F: FnMut(&K, &mut V) -> bool, + { + DrainFilter { pred, inner: self.drain_filter_inner() } + } + pub(super) fn drain_filter_inner(&mut self) -> DrainFilterInner<'_, K, V> { + let front = self.root.as_mut().map(|r| r.as_mut().first_leaf_edge()); + DrainFilterInner { length: &mut self.length, cur_leaf_edge: front } + } + /// Calculates the number of elements if it is incorrect. fn recalc_length(&mut self) { fn dfs<'a, K, V>(node: NodeRef, K, V, marker::LeafOrInternal>) -> usize @@ -1653,6 +1695,124 @@ impl Clone for Values<'_, K, V> { } } +/// An iterator produced by calling `drain_filter` on BTreeMap. +#[unstable(feature = "btree_drain_filter", issue = "70530")] +pub struct DrainFilter<'a, K, V, F> +where + K: 'a + Ord, // This Ord bound should be removed before stabilization. + V: 'a, + F: 'a + FnMut(&K, &mut V) -> bool, +{ + pred: F, + inner: DrainFilterInner<'a, K, V>, +} +pub(super) struct DrainFilterInner<'a, K, V> +where + K: 'a + Ord, + V: 'a, +{ + length: &'a mut usize, + cur_leaf_edge: Option, K, V, marker::Leaf>, marker::Edge>>, +} + +#[unstable(feature = "btree_drain_filter", issue = "70530")] +impl<'a, K, V, F> Drop for DrainFilter<'a, K, V, F> +where + K: 'a + Ord, + V: 'a, + F: 'a + FnMut(&K, &mut V) -> bool, +{ + fn drop(&mut self) { + self.for_each(drop); + } +} + +#[unstable(feature = "btree_drain_filter", issue = "70530")] +impl<'a, K, V, F> fmt::Debug for DrainFilter<'a, K, V, F> +where + K: 'a + fmt::Debug + Ord, + V: 'a + fmt::Debug, + F: 'a + FnMut(&K, &mut V) -> bool, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_tuple("DrainFilter").field(&self.inner.peek()).finish() + } +} + +#[unstable(feature = "btree_drain_filter", issue = "70530")] +impl<'a, K, V, F> Iterator for DrainFilter<'a, K, V, F> +where + K: 'a + Ord, + V: 'a, + F: 'a + FnMut(&K, &mut V) -> bool, +{ + type Item = (K, V); + + fn next(&mut self) -> Option<(K, V)> { + self.inner.next(&mut self.pred) + } + + fn size_hint(&self) -> (usize, Option) { + self.inner.size_hint() + } +} + +impl<'a, K, V> DrainFilterInner<'a, K, V> +where + K: 'a + Ord, + V: 'a, +{ + /// Allow Debug implementations to predict the next element. + pub(super) fn peek(&self) -> Option<(&K, &V)> { + let edge = self.cur_leaf_edge.as_ref()?; + edge.reborrow().next_kv().ok().map(|kv| kv.into_kv()) + } + + unsafe fn next_kv( + &mut self, + ) -> Option, K, V, marker::LeafOrInternal>, marker::KV>> { + let edge = self.cur_leaf_edge.as_ref()?; + ptr::read(edge).next_kv().ok() + } + + /// Implementation of a typical `DrainFilter::next` method, given the predicate. + pub(super) fn next(&mut self, pred: &mut F) -> Option<(K, V)> + where + F: FnMut(&K, &mut V) -> bool, + { + while let Some(kv) = unsafe { self.next_kv() } { + let (k, v) = unsafe { ptr::read(&kv) }.into_kv_mut(); + if pred(k, v) { + *self.length -= 1; + let (k, v, leaf_edge_location) = kv.remove_kv_tracking(); + // `remove_kv_tracking` has either preserved or invalidated `self.cur_leaf_edge` + if let Some(node) = leaf_edge_location { + match search::search_tree(node, &k) { + search::SearchResult::Found(_) => unreachable!(), + search::SearchResult::GoDown(leaf) => self.cur_leaf_edge = Some(leaf), + } + }; + return Some((k, v)); + } + self.cur_leaf_edge = Some(kv.next_leaf_edge()); + } + None + } + + /// Implementation of a typical `DrainFilter::size_hint` method. + pub(super) fn size_hint(&self) -> (usize, Option) { + (0, Some(*self.length)) + } +} + +#[unstable(feature = "btree_drain_filter", issue = "70530")] +impl FusedIterator for DrainFilter<'_, K, V, F> +where + K: Ord, + F: FnMut(&K, &mut V) -> bool, +{ +} + #[stable(feature = "btree_range", since = "1.17.0")] impl<'a, K, V> Iterator for Range<'a, K, V> { type Item = (&'a K, &'a V); @@ -2531,12 +2691,31 @@ impl<'a, K: Ord, V> OccupiedEntry<'a, K, V> { fn remove_kv(self) -> (K, V) { *self.length -= 1; - let (small_leaf, old_key, old_val) = match self.handle.force() { + let (old_key, old_val, _) = self.handle.remove_kv_tracking(); + (old_key, old_val) + } +} + +impl<'a, K: 'a, V: 'a> Handle, K, V, marker::LeafOrInternal>, marker::KV> { + /// Removes a key/value-pair from the map, and returns that pair, as well as + /// the whereabouts of the leaf edge corresponding to that former pair: + /// if None is returned, the leaf edge is still the left leaf edge of the KV handle; + /// if a node is returned, it heads the subtree where the leaf edge may be found. + fn remove_kv_tracking( + self, + ) -> (K, V, Option, K, V, marker::LeafOrInternal>>) { + let mut levels_down_handled: isize; + let (small_leaf, old_key, old_val) = match self.force() { Leaf(leaf) => { + levels_down_handled = 1; // handled at same level, but affects only the right side let (hole, old_key, old_val) = leaf.remove(); (hole.into_node(), old_key, old_val) } Internal(mut internal) => { + // Replace the location freed in the internal node with the next KV, + // and remove that next KV from its leaf. + levels_down_handled = unsafe { ptr::read(&internal).into_node().height() } as isize; + let key_loc = internal.kv_mut().0 as *mut K; let val_loc = internal.kv_mut().1 as *mut V; @@ -2556,27 +2735,39 @@ impl<'a, K: Ord, V> OccupiedEntry<'a, K, V> { let mut cur_node = small_leaf.forget_type(); while cur_node.len() < node::MIN_LEN { match handle_underfull_node(cur_node) { - AtRoot => break, + AtRoot(root) => { + cur_node = root; + break; + } EmptyParent(_) => unreachable!(), Merged(parent) => { + levels_down_handled -= 1; if parent.len() == 0 { // We must be at the root - parent.into_root_mut().pop_level(); + let root = parent.into_root_mut(); + root.pop_level(); + cur_node = root.as_mut(); break; } else { cur_node = parent.forget_type(); } } - Stole(_) => break, + Stole(internal_node) => { + levels_down_handled -= 1; + cur_node = internal_node.forget_type(); + // This internal node might be underfull, but only if it's the root. + break; + } } } - (old_key, old_val) + let leaf_edge_location = if levels_down_handled > 0 { None } else { Some(cur_node) }; + (old_key, old_val, leaf_edge_location) } } enum UnderflowResult<'a, K, V> { - AtRoot, + AtRoot(NodeRef, K, V, marker::LeafOrInternal>), EmptyParent(NodeRef, K, V, marker::Internal>), Merged(NodeRef, K, V, marker::Internal>), Stole(NodeRef, K, V, marker::Internal>), @@ -2585,10 +2776,9 @@ enum UnderflowResult<'a, K, V> { fn handle_underfull_node( node: NodeRef, K, V, marker::LeafOrInternal>, ) -> UnderflowResult<'_, K, V> { - let parent = if let Ok(parent) = node.ascend() { - parent - } else { - return AtRoot; + let parent = match node.ascend() { + Ok(parent) => parent, + Err(root) => return AtRoot(root), }; let (is_left, mut handle) = match parent.left_kv() { diff --git a/src/liballoc/collections/btree/set.rs b/src/liballoc/collections/btree/set.rs index b100ce754caad..0b02223def4f8 100644 --- a/src/liballoc/collections/btree/set.rs +++ b/src/liballoc/collections/btree/set.rs @@ -8,8 +8,8 @@ use core::fmt::{self, Debug}; use core::iter::{FromIterator, FusedIterator, Peekable}; use core::ops::{BitAnd, BitOr, BitXor, RangeBounds, Sub}; +use super::map::{BTreeMap, Keys}; use super::Recover; -use crate::collections::btree_map::{self, BTreeMap, Keys}; // FIXME(conventions): implement bounded iterators @@ -102,7 +102,7 @@ impl fmt::Debug for Iter<'_, T> { #[stable(feature = "rust1", since = "1.0.0")] #[derive(Debug)] pub struct IntoIter { - iter: btree_map::IntoIter, + iter: super::map::IntoIter, } /// An iterator over a sub-range of items in a `BTreeSet`. @@ -115,7 +115,7 @@ pub struct IntoIter { #[derive(Debug)] #[stable(feature = "btree_range", since = "1.17.0")] pub struct Range<'a, T: 'a> { - iter: btree_map::Range<'a, T, ()>, + iter: super::map::Range<'a, T, ()>, } /// Core of SymmetricDifference and Union. @@ -944,6 +944,41 @@ impl BTreeSet { { BTreeSet { map: self.map.split_off(key) } } + + /// Creates an iterator which uses a closure to determine if a value should be removed. + /// + /// If the closure returns true, then the value is removed and yielded. + /// If the closure returns false, the value will remain in the list and will not be yielded + /// by the iterator. + /// + /// If the iterator is only partially consumed or not consumed at all, each of the remaining + /// values will still be subjected to the closure and removed and dropped if it returns true. + /// + /// It is unspecified how many more values will be subjected to the closure + /// if a panic occurs in the closure, or if a panic occurs while dropping a value, or if the + /// `DrainFilter` itself is leaked. + /// + /// # Examples + /// + /// Splitting a set into even and odd values, reusing the original set: + /// + /// ``` + /// #![feature(btree_drain_filter)] + /// use std::collections::BTreeSet; + /// + /// let mut set: BTreeSet = (0..8).collect(); + /// let evens: BTreeSet<_> = set.drain_filter(|v| v % 2 == 0).collect(); + /// let odds = set; + /// assert_eq!(evens.into_iter().collect::>(), vec![0, 2, 4, 6]); + /// assert_eq!(odds.into_iter().collect::>(), vec![1, 3, 5, 7]); + /// ``` + #[unstable(feature = "btree_drain_filter", issue = "70530")] + pub fn drain_filter<'a, F>(&'a mut self, pred: F) -> DrainFilter<'a, T, F> + where + F: 'a + FnMut(&T) -> bool, + { + DrainFilter { pred, inner: self.map.drain_filter_inner() } + } } impl BTreeSet { @@ -1055,6 +1090,66 @@ impl<'a, T> IntoIterator for &'a BTreeSet { } } +/// An iterator produced by calling `drain_filter` on BTreeSet. +#[unstable(feature = "btree_drain_filter", issue = "70530")] +pub struct DrainFilter<'a, T, F> +where + T: 'a + Ord, + F: 'a + FnMut(&T) -> bool, +{ + pred: F, + inner: super::map::DrainFilterInner<'a, T, ()>, +} + +#[unstable(feature = "btree_drain_filter", issue = "70530")] +impl<'a, T, F> Drop for DrainFilter<'a, T, F> +where + T: 'a + Ord, + F: 'a + FnMut(&T) -> bool, +{ + fn drop(&mut self) { + self.for_each(drop); + } +} + +#[unstable(feature = "btree_drain_filter", issue = "70530")] +impl<'a, T, F> fmt::Debug for DrainFilter<'a, T, F> +where + T: 'a + Ord + fmt::Debug, + F: 'a + FnMut(&T) -> bool, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_tuple("DrainFilter").field(&self.inner.peek().map(|(k, _)| k)).finish() + } +} + +#[unstable(feature = "btree_drain_filter", issue = "70530")] +impl<'a, 'f, T, F> Iterator for DrainFilter<'a, T, F> +where + T: 'a + Ord, + F: 'a + 'f + FnMut(&T) -> bool, +{ + type Item = T; + + fn next(&mut self) -> Option { + let pred = &mut self.pred; + let mut mapped_pred = |k: &T, _v: &mut ()| pred(k); + self.inner.next(&mut mapped_pred).map(|(k, _)| k) + } + + fn size_hint(&self) -> (usize, Option) { + self.inner.size_hint() + } +} + +#[unstable(feature = "btree_drain_filter", issue = "70530")] +impl<'a, T, F> FusedIterator for DrainFilter<'a, T, F> +where + T: 'a + Ord, + F: 'a + FnMut(&T) -> bool, +{ +} + #[stable(feature = "rust1", since = "1.0.0")] impl Extend for BTreeSet { #[inline] diff --git a/src/liballoc/tests/btree/map.rs b/src/liballoc/tests/btree/map.rs index 535b6a9c31451..5f158cb4c638c 100644 --- a/src/liballoc/tests/btree/map.rs +++ b/src/liballoc/tests/btree/map.rs @@ -5,7 +5,7 @@ use std::fmt::Debug; use std::iter::FromIterator; use std::ops::Bound::{self, Excluded, Included, Unbounded}; use std::ops::RangeBounds; -use std::panic::catch_unwind; +use std::panic::{catch_unwind, AssertUnwindSafe}; use std::rc::Rc; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -609,6 +609,263 @@ fn test_range_mut() { } } +mod test_drain_filter { + use super::*; + + #[test] + fn empty() { + let mut map: BTreeMap = BTreeMap::new(); + map.drain_filter(|_, _| unreachable!("there's nothing to decide on")); + assert!(map.is_empty()); + } + + #[test] + fn consuming_nothing() { + let pairs = (0..3).map(|i| (i, i)); + let mut map: BTreeMap<_, _> = pairs.collect(); + assert!(map.drain_filter(|_, _| false).eq(std::iter::empty())); + } + + #[test] + fn consuming_all() { + let pairs = (0..3).map(|i| (i, i)); + let mut map: BTreeMap<_, _> = pairs.clone().collect(); + assert!(map.drain_filter(|_, _| true).eq(pairs)); + } + + #[test] + fn mutating_and_keeping() { + let pairs = (0..3).map(|i| (i, i)); + let mut map: BTreeMap<_, _> = pairs.collect(); + assert!( + map.drain_filter(|_, v| { + *v += 6; + false + }) + .eq(std::iter::empty()) + ); + assert!(map.keys().copied().eq(0..3)); + assert!(map.values().copied().eq(6..9)); + } + + #[test] + fn mutating_and_removing() { + let pairs = (0..3).map(|i| (i, i)); + let mut map: BTreeMap<_, _> = pairs.collect(); + assert!( + map.drain_filter(|_, v| { + *v += 6; + true + }) + .eq((0..3).map(|i| (i, i + 6))) + ); + assert!(map.is_empty()); + } + + #[test] + fn underfull_keeping_all() { + let pairs = (0..3).map(|i| (i, i)); + let mut map: BTreeMap<_, _> = pairs.collect(); + map.drain_filter(|_, _| false); + assert!(map.keys().copied().eq(0..3)); + } + + #[test] + fn underfull_removing_one() { + let pairs = (0..3).map(|i| (i, i)); + for doomed in 0..3 { + let mut map: BTreeMap<_, _> = pairs.clone().collect(); + map.drain_filter(|i, _| *i == doomed); + assert_eq!(map.len(), 2); + } + } + + #[test] + fn underfull_keeping_one() { + let pairs = (0..3).map(|i| (i, i)); + for sacred in 0..3 { + let mut map: BTreeMap<_, _> = pairs.clone().collect(); + map.drain_filter(|i, _| *i != sacred); + assert!(map.keys().copied().eq(sacred..=sacred)); + } + } + + #[test] + fn underfull_removing_all() { + let pairs = (0..3).map(|i| (i, i)); + let mut map: BTreeMap<_, _> = pairs.collect(); + map.drain_filter(|_, _| true); + assert!(map.is_empty()); + } + + #[test] + fn height_0_keeping_all() { + let pairs = (0..NODE_CAPACITY).map(|i| (i, i)); + let mut map: BTreeMap<_, _> = pairs.collect(); + map.drain_filter(|_, _| false); + assert!(map.keys().copied().eq(0..NODE_CAPACITY)); + } + + #[test] + fn height_0_removing_one() { + let pairs = (0..NODE_CAPACITY).map(|i| (i, i)); + for doomed in 0..NODE_CAPACITY { + let mut map: BTreeMap<_, _> = pairs.clone().collect(); + map.drain_filter(|i, _| *i == doomed); + assert_eq!(map.len(), NODE_CAPACITY - 1); + } + } + + #[test] + fn height_0_keeping_one() { + let pairs = (0..NODE_CAPACITY).map(|i| (i, i)); + for sacred in 0..NODE_CAPACITY { + let mut map: BTreeMap<_, _> = pairs.clone().collect(); + map.drain_filter(|i, _| *i != sacred); + assert!(map.keys().copied().eq(sacred..=sacred)); + } + } + + #[test] + fn height_0_removing_all() { + let pairs = (0..NODE_CAPACITY).map(|i| (i, i)); + let mut map: BTreeMap<_, _> = pairs.collect(); + map.drain_filter(|_, _| true); + assert!(map.is_empty()); + } + + #[test] + fn height_0_keeping_half() { + let mut map: BTreeMap<_, _> = (0..16).map(|i| (i, i)).collect(); + assert_eq!(map.drain_filter(|i, _| *i % 2 == 0).count(), 8); + assert_eq!(map.len(), 8); + } + + #[test] + fn height_1_removing_all() { + let pairs = (0..MIN_INSERTS_HEIGHT_1).map(|i| (i, i)); + let mut map: BTreeMap<_, _> = pairs.collect(); + map.drain_filter(|_, _| true); + assert!(map.is_empty()); + } + + #[test] + fn height_1_removing_one() { + let pairs = (0..MIN_INSERTS_HEIGHT_1).map(|i| (i, i)); + for doomed in 0..MIN_INSERTS_HEIGHT_1 { + let mut map: BTreeMap<_, _> = pairs.clone().collect(); + map.drain_filter(|i, _| *i == doomed); + assert_eq!(map.len(), MIN_INSERTS_HEIGHT_1 - 1); + } + } + + #[test] + fn height_1_keeping_one() { + let pairs = (0..MIN_INSERTS_HEIGHT_1).map(|i| (i, i)); + for sacred in 0..MIN_INSERTS_HEIGHT_1 { + let mut map: BTreeMap<_, _> = pairs.clone().collect(); + map.drain_filter(|i, _| *i != sacred); + assert!(map.keys().copied().eq(sacred..=sacred)); + } + } + + #[cfg(not(miri))] // Miri is too slow + #[test] + fn height_2_removing_one() { + let pairs = (0..MIN_INSERTS_HEIGHT_2).map(|i| (i, i)); + for doomed in (0..MIN_INSERTS_HEIGHT_2).step_by(12) { + let mut map: BTreeMap<_, _> = pairs.clone().collect(); + map.drain_filter(|i, _| *i == doomed); + assert_eq!(map.len(), MIN_INSERTS_HEIGHT_2 - 1); + } + } + + #[cfg(not(miri))] // Miri is too slow + #[test] + fn height_2_keeping_one() { + let pairs = (0..MIN_INSERTS_HEIGHT_2).map(|i| (i, i)); + for sacred in (0..MIN_INSERTS_HEIGHT_2).step_by(12) { + let mut map: BTreeMap<_, _> = pairs.clone().collect(); + map.drain_filter(|i, _| *i != sacred); + assert!(map.keys().copied().eq(sacred..=sacred)); + } + } + + #[test] + fn height_2_removing_all() { + let pairs = (0..MIN_INSERTS_HEIGHT_2).map(|i| (i, i)); + let mut map: BTreeMap<_, _> = pairs.collect(); + map.drain_filter(|_, _| true); + assert!(map.is_empty()); + } + + #[test] + fn drop_panic_leak() { + static PREDS: AtomicUsize = AtomicUsize::new(0); + static DROPS: AtomicUsize = AtomicUsize::new(0); + + struct D; + impl Drop for D { + fn drop(&mut self) { + if DROPS.fetch_add(1, Ordering::SeqCst) == 1 { + panic!("panic in `drop`"); + } + } + } + + let mut map = BTreeMap::new(); + map.insert(0, D); + map.insert(4, D); + map.insert(8, D); + + catch_unwind(move || { + drop(map.drain_filter(|i, _| { + PREDS.fetch_add(1usize << i, Ordering::SeqCst); + true + })) + }) + .ok(); + + assert_eq!(PREDS.load(Ordering::SeqCst), 0x011); + assert_eq!(DROPS.load(Ordering::SeqCst), 3); + } + + #[test] + fn pred_panic_leak() { + static PREDS: AtomicUsize = AtomicUsize::new(0); + static DROPS: AtomicUsize = AtomicUsize::new(0); + + struct D; + impl Drop for D { + fn drop(&mut self) { + DROPS.fetch_add(1, Ordering::SeqCst); + } + } + + let mut map = BTreeMap::new(); + map.insert(0, D); + map.insert(4, D); + map.insert(8, D); + + catch_unwind(AssertUnwindSafe(|| { + drop(map.drain_filter(|i, _| { + PREDS.fetch_add(1usize << i, Ordering::SeqCst); + match i { + 0 => true, + _ => panic!(), + } + })) + })) + .ok(); + + assert_eq!(PREDS.load(Ordering::SeqCst), 0x011); + assert_eq!(DROPS.load(Ordering::SeqCst), 1); + assert_eq!(map.len(), 2); + assert_eq!(map.first_entry().unwrap().key(), &4); + assert_eq!(map.last_entry().unwrap().key(), &8); + } +} + #[test] fn test_borrow() { // make sure these compile -- using the Borrow trait diff --git a/src/liballoc/tests/btree/set.rs b/src/liballoc/tests/btree/set.rs index 1a2b62d026b2e..136018b9f7df5 100644 --- a/src/liballoc/tests/btree/set.rs +++ b/src/liballoc/tests/btree/set.rs @@ -1,5 +1,7 @@ use std::collections::BTreeSet; use std::iter::FromIterator; +use std::panic::{catch_unwind, AssertUnwindSafe}; +use std::sync::atomic::{AtomicU32, Ordering}; use super::DeterministicRng; @@ -302,6 +304,85 @@ fn test_is_subset() { assert_eq!(is_subset(&[99, 100], &large), false); } +#[test] +fn test_drain_filter() { + let mut x: BTreeSet<_> = [1].iter().copied().collect(); + let mut y: BTreeSet<_> = [1].iter().copied().collect(); + + x.drain_filter(|_| true); + y.drain_filter(|_| false); + assert_eq!(x.len(), 0); + assert_eq!(y.len(), 1); +} + +#[test] +fn test_drain_filter_drop_panic_leak() { + static PREDS: AtomicU32 = AtomicU32::new(0); + static DROPS: AtomicU32 = AtomicU32::new(0); + + #[derive(PartialEq, Eq, PartialOrd, Ord)] + struct D(i32); + impl Drop for D { + fn drop(&mut self) { + if DROPS.fetch_add(1, Ordering::SeqCst) == 1 { + panic!("panic in `drop`"); + } + } + } + + let mut set = BTreeSet::new(); + set.insert(D(0)); + set.insert(D(4)); + set.insert(D(8)); + + catch_unwind(move || { + drop(set.drain_filter(|d| { + PREDS.fetch_add(1u32 << d.0, Ordering::SeqCst); + true + })) + }) + .ok(); + + assert_eq!(PREDS.load(Ordering::SeqCst), 0x011); + assert_eq!(DROPS.load(Ordering::SeqCst), 3); +} + +#[test] +fn test_drain_filter_pred_panic_leak() { + static PREDS: AtomicU32 = AtomicU32::new(0); + static DROPS: AtomicU32 = AtomicU32::new(0); + + #[derive(PartialEq, Eq, PartialOrd, Ord)] + struct D(i32); + impl Drop for D { + fn drop(&mut self) { + DROPS.fetch_add(1, Ordering::SeqCst); + } + } + + let mut set = BTreeSet::new(); + set.insert(D(0)); + set.insert(D(4)); + set.insert(D(8)); + + catch_unwind(AssertUnwindSafe(|| { + drop(set.drain_filter(|d| { + PREDS.fetch_add(1u32 << d.0, Ordering::SeqCst); + match d.0 { + 0 => true, + _ => panic!(), + } + })) + })) + .ok(); + + assert_eq!(PREDS.load(Ordering::SeqCst), 0x011); + assert_eq!(DROPS.load(Ordering::SeqCst), 1); + assert_eq!(set.len(), 2); + assert_eq!(set.first().unwrap().0, 4); + assert_eq!(set.last().unwrap().0, 8); +} + #[test] fn test_clear() { let mut x = BTreeSet::new(); diff --git a/src/liballoc/tests/lib.rs b/src/liballoc/tests/lib.rs index ea75f8903c368..ad6feaeebc67f 100644 --- a/src/liballoc/tests/lib.rs +++ b/src/liballoc/tests/lib.rs @@ -1,5 +1,6 @@ #![feature(allocator_api)] #![feature(box_syntax)] +#![feature(btree_drain_filter)] #![feature(drain_filter)] #![feature(exact_size_is_empty)] #![feature(map_first_last)] From 96d73536782fb4a6a4b98db330ac5446c63a8005 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 30 Mar 2020 09:20:55 +0200 Subject: [PATCH 05/12] parse_and_disallow_postfix_after_cast: account for `ExprKind::Err`. --- src/librustc_parse/parser/expr.rs | 1 + .../parser/issue-70552-ascription-in-parens-after-call.rs | 3 +++ .../issue-70552-ascription-in-parens-after-call.stderr | 8 ++++++++ 3 files changed, 12 insertions(+) create mode 100644 src/test/ui/parser/issue-70552-ascription-in-parens-after-call.rs create mode 100644 src/test/ui/parser/issue-70552-ascription-in-parens-after-call.stderr diff --git a/src/librustc_parse/parser/expr.rs b/src/librustc_parse/parser/expr.rs index b205a4b322229..cbff99f8da612 100644 --- a/src/librustc_parse/parser/expr.rs +++ b/src/librustc_parse/parser/expr.rs @@ -638,6 +638,7 @@ impl<'a> Parser<'a> { ExprKind::MethodCall(_, _) => "a method call", ExprKind::Call(_, _) => "a function call", ExprKind::Await(_) => "`.await`", + ExprKind::Err => return Ok(with_postfix), _ => unreachable!("parse_dot_or_call_expr_with_ shouldn't produce this"), } ); diff --git a/src/test/ui/parser/issue-70552-ascription-in-parens-after-call.rs b/src/test/ui/parser/issue-70552-ascription-in-parens-after-call.rs new file mode 100644 index 0000000000000..9b6dd7db4beb3 --- /dev/null +++ b/src/test/ui/parser/issue-70552-ascription-in-parens-after-call.rs @@ -0,0 +1,3 @@ +fn main() { + expr as fun()(:); //~ ERROR expected expression +} diff --git a/src/test/ui/parser/issue-70552-ascription-in-parens-after-call.stderr b/src/test/ui/parser/issue-70552-ascription-in-parens-after-call.stderr new file mode 100644 index 0000000000000..f03c92e1b1f17 --- /dev/null +++ b/src/test/ui/parser/issue-70552-ascription-in-parens-after-call.stderr @@ -0,0 +1,8 @@ +error: expected expression, found `:` + --> $DIR/issue-70552-ascription-in-parens-after-call.rs:2:19 + | +LL | expr as fun()(:); + | ^ expected expression + +error: aborting due to previous error + From 1ae3b5022bf767161ca336f718cf823e3147e419 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Tue, 31 Mar 2020 12:27:09 +0100 Subject: [PATCH 06/12] Add missing -lmsvcrt on mingw after -lpthread Fixes #70316 --- src/librustc_target/spec/windows_base.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/librustc_target/spec/windows_base.rs b/src/librustc_target/spec/windows_base.rs index 188548b41fe75..5f59ca8a5a311 100644 --- a/src/librustc_target/spec/windows_base.rs +++ b/src/librustc_target/spec/windows_base.rs @@ -60,6 +60,8 @@ pub fn opts() -> TargetOptions { "-lgcc".to_string(), "-lgcc_eh".to_string(), "-lpthread".to_string(), + // libpthread depends on libmsvcrt, so we need to link it *again*. + "-lmsvcrt".to_string(), "-lkernel32".to_string(), ], ); From 21c5ccab10b30085479a7189ee555f77c47c79e7 Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Fri, 27 Mar 2020 21:55:15 +0100 Subject: [PATCH 07/12] fix internal lint fallout --- src/libcore/array/iter.rs | 18 +++++++++--------- src/librustc_macros/src/query.rs | 4 +++- src/librustc_resolve/lib.rs | 2 +- src/libstd/sys/windows/handle.rs | 5 ++--- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/libcore/array/iter.rs b/src/libcore/array/iter.rs index 80eaae0d4afb5..f6b8d4ba08146 100644 --- a/src/libcore/array/iter.rs +++ b/src/libcore/array/iter.rs @@ -39,7 +39,7 @@ where alive: Range, } -impl IntoIter +impl IntoIter where [T; N]: LengthAtMost32, { @@ -99,7 +99,7 @@ where } #[stable(feature = "array_value_iter_impls", since = "1.40.0")] -impl Iterator for IntoIter +impl Iterator for IntoIter where [T; N]: LengthAtMost32, { @@ -146,7 +146,7 @@ where } #[stable(feature = "array_value_iter_impls", since = "1.40.0")] -impl DoubleEndedIterator for IntoIter +impl DoubleEndedIterator for IntoIter where [T; N]: LengthAtMost32, { @@ -182,7 +182,7 @@ where } #[stable(feature = "array_value_iter_impls", since = "1.40.0")] -impl Drop for IntoIter +impl Drop for IntoIter where [T; N]: LengthAtMost32, { @@ -195,7 +195,7 @@ where } #[stable(feature = "array_value_iter_impls", since = "1.40.0")] -impl ExactSizeIterator for IntoIter +impl ExactSizeIterator for IntoIter where [T; N]: LengthAtMost32, { @@ -210,17 +210,17 @@ where } #[stable(feature = "array_value_iter_impls", since = "1.40.0")] -impl FusedIterator for IntoIter where [T; N]: LengthAtMost32 {} +impl FusedIterator for IntoIter where [T; N]: LengthAtMost32 {} // The iterator indeed reports the correct length. The number of "alive" // elements (that will still be yielded) is the length of the range `alive`. // This range is decremented in length in either `next` or `next_back`. It is // always decremented by 1 in those methods, but only if `Some(_)` is returned. #[stable(feature = "array_value_iter_impls", since = "1.40.0")] -unsafe impl TrustedLen for IntoIter where [T; N]: LengthAtMost32 {} +unsafe impl TrustedLen for IntoIter where [T; N]: LengthAtMost32 {} #[stable(feature = "array_value_iter_impls", since = "1.40.0")] -impl Clone for IntoIter +impl Clone for IntoIter where [T; N]: LengthAtMost32, { @@ -249,7 +249,7 @@ where } #[stable(feature = "array_value_iter_impls", since = "1.40.0")] -impl fmt::Debug for IntoIter +impl fmt::Debug for IntoIter where [T; N]: LengthAtMost32, { diff --git a/src/librustc_macros/src/query.rs b/src/librustc_macros/src/query.rs index e7005f2f5ba77..33bcd9456c33b 100644 --- a/src/librustc_macros/src/query.rs +++ b/src/librustc_macros/src/query.rs @@ -356,9 +356,11 @@ fn add_query_description_impl( quote! { #t } }) .unwrap_or(quote! { _ }); + // expr is a `Block`, meaning that `{ #expr }` gets expanded + // to `{ { stmts... } }`, which triggers the `unused_braces` lint. quote! { #[inline] - #[allow(unused_variables)] + #[allow(unused_variables, unused_braces)] fn cache_on_disk( #tcx: TyCtxt<'tcx>, #key: Self::Key, diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 9d5121cbad562..aba1567e6ef6b 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -2807,7 +2807,7 @@ impl<'a> Resolver<'a> { ast::Path { span, segments: iter::once(Ident::with_dummy_span(kw::PathRoot)) - .chain({ path_str.split("::").skip(1).map(Ident::from_str) }) + .chain(path_str.split("::").skip(1).map(Ident::from_str)) .map(|i| self.new_ast_path_segment(i)) .collect(), } diff --git a/src/libstd/sys/windows/handle.rs b/src/libstd/sys/windows/handle.rs index f2ad057b6b624..d00381792e351 100644 --- a/src/libstd/sys/windows/handle.rs +++ b/src/libstd/sys/windows/handle.rs @@ -115,8 +115,7 @@ impl RawHandle { ) -> io::Result> { let len = cmp::min(buf.len(), ::max_value() as usize) as c::DWORD; let mut amt = 0; - let res = - cvt({ c::ReadFile(self.0, buf.as_ptr() as c::LPVOID, len, &mut amt, overlapped) }); + let res = cvt(c::ReadFile(self.0, buf.as_ptr() as c::LPVOID, len, &mut amt, overlapped)); match res { Ok(_) => Ok(Some(amt as usize)), Err(e) => { @@ -139,7 +138,7 @@ impl RawHandle { unsafe { let mut bytes = 0; let wait = if wait { c::TRUE } else { c::FALSE }; - let res = cvt({ c::GetOverlappedResult(self.raw(), overlapped, &mut bytes, wait) }); + let res = cvt(c::GetOverlappedResult(self.raw(), overlapped, &mut bytes, wait)); match res { Ok(_) => Ok(bytes as usize), Err(e) => { From 698b20eedaff60aaed3b1f0bd1bec7d8c095f6a4 Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Fri, 27 Mar 2020 21:56:19 +0100 Subject: [PATCH 08/12] update tests --- src/test/incremental/const-generics/issue-61516.rs | 2 +- src/test/ui/array-slice-vec/vec-fixed-length.rs | 2 +- src/test/ui/block-fn-coerce.rs | 1 + src/test/ui/cleanup-rvalue-scopes.rs | 2 +- src/test/ui/coerce/coerce-expect-unsized.rs | 1 + src/test/ui/coerce/coerce-overloaded-autoderef.rs | 1 + src/test/ui/const-generics/issues/issue-62504.rs | 4 ++-- src/test/ui/const-generics/issues/issue-70125-2.rs | 2 +- src/test/ui/consts/const-block.rs | 2 +- src/test/ui/expr-block-generic-unique1.rs | 2 +- src/test/ui/expr-block-generic-unique2.rs | 2 +- src/test/ui/expr-block-generic.rs | 1 + src/test/ui/expr-block-unique.rs | 2 +- src/test/ui/expr-block.rs | 5 +---- src/test/ui/expr-fn.rs | 1 + src/test/ui/functions-closures/closure-inference.rs | 2 +- src/test/ui/functions-closures/closure-inference2.rs | 2 +- src/test/ui/intrinsics/intrinsic-move-val-cleanups.rs | 1 + src/test/ui/issues/issue-23898.rs | 1 + src/test/ui/issues/issue-28777.rs | 1 + src/test/ui/range.rs | 2 +- src/test/ui/range_inclusive.rs | 2 +- src/test/ui/structs-enums/empty-tag.rs | 1 + src/test/ui/type-alias-impl-trait/assoc-type-const.rs | 8 ++++---- src/test/ui/unsized-locals/unsized-exprs-rpass.rs | 3 +-- src/test/ui/weird-exprs.rs | 2 +- src/test/ui/zero-sized/zero-sized-tuple-struct.rs | 1 + 27 files changed, 31 insertions(+), 25 deletions(-) diff --git a/src/test/incremental/const-generics/issue-61516.rs b/src/test/incremental/const-generics/issue-61516.rs index a7465b77267a5..a193bf998dc73 100644 --- a/src/test/incremental/const-generics/issue-61516.rs +++ b/src/test/incremental/const-generics/issue-61516.rs @@ -4,7 +4,7 @@ struct FakeArray(T); -impl FakeArray { +impl FakeArray { fn len(&self) -> usize { N } diff --git a/src/test/ui/array-slice-vec/vec-fixed-length.rs b/src/test/ui/array-slice-vec/vec-fixed-length.rs index 5db02ee066bbe..908c39c7951c0 100644 --- a/src/test/ui/array-slice-vec/vec-fixed-length.rs +++ b/src/test/ui/array-slice-vec/vec-fixed-length.rs @@ -9,7 +9,7 @@ fn test_big_vec() {} #[cfg(target_pointer_width = "64")] fn test_big_vec() { - assert_eq!(size_of::<[u8; (1 << 32)]>(), (1 << 32)); + assert_eq!(size_of::<[u8; 1 << 32]>(), (1 << 32)); } fn main() { diff --git a/src/test/ui/block-fn-coerce.rs b/src/test/ui/block-fn-coerce.rs index fc5f51d46b255..d993ad9945974 100644 --- a/src/test/ui/block-fn-coerce.rs +++ b/src/test/ui/block-fn-coerce.rs @@ -1,4 +1,5 @@ // run-pass +#![allow(unused_braces)] fn force(f: F) -> isize where F: FnOnce() -> isize { return f(); } diff --git a/src/test/ui/cleanup-rvalue-scopes.rs b/src/test/ui/cleanup-rvalue-scopes.rs index f51f13abf792f..c5dd87c0f5a1f 100644 --- a/src/test/ui/cleanup-rvalue-scopes.rs +++ b/src/test/ui/cleanup-rvalue-scopes.rs @@ -1,5 +1,5 @@ // run-pass - +#![allow(unused_braces)] #![allow(non_snake_case)] #![allow(unused_variables)] // Test that destructors for rvalue temporaries run either at end of diff --git a/src/test/ui/coerce/coerce-expect-unsized.rs b/src/test/ui/coerce/coerce-expect-unsized.rs index b44aa6ab37760..d486fdf73aba8 100644 --- a/src/test/ui/coerce/coerce-expect-unsized.rs +++ b/src/test/ui/coerce/coerce-expect-unsized.rs @@ -1,4 +1,5 @@ // run-pass +#![allow(unused_braces)] #![feature(box_syntax)] use std::cell::RefCell; diff --git a/src/test/ui/coerce/coerce-overloaded-autoderef.rs b/src/test/ui/coerce/coerce-overloaded-autoderef.rs index 3fe18103ef8c3..d5484607c8b52 100644 --- a/src/test/ui/coerce/coerce-overloaded-autoderef.rs +++ b/src/test/ui/coerce/coerce-overloaded-autoderef.rs @@ -1,4 +1,5 @@ // run-pass +#![allow(unused_braces)] #![allow(dead_code)] // pretty-expanded FIXME #23616 diff --git a/src/test/ui/const-generics/issues/issue-62504.rs b/src/test/ui/const-generics/issues/issue-62504.rs index cd3cfaac3b95b..212e16253f6b8 100644 --- a/src/test/ui/const-generics/issues/issue-62504.rs +++ b/src/test/ui/const-generics/issues/issue-62504.rs @@ -7,13 +7,13 @@ trait HasSize { const SIZE: usize; } -impl HasSize for ArrayHolder<{ X }> { +impl HasSize for ArrayHolder { const SIZE: usize = X; } struct ArrayHolder([u32; X]); -impl ArrayHolder<{ X }> { +impl ArrayHolder { pub const fn new() -> Self { ArrayHolder([0; Self::SIZE]) //~^ ERROR: mismatched types diff --git a/src/test/ui/const-generics/issues/issue-70125-2.rs b/src/test/ui/const-generics/issues/issue-70125-2.rs index ea7a68c2f93d9..a3eca0dd7d965 100644 --- a/src/test/ui/const-generics/issues/issue-70125-2.rs +++ b/src/test/ui/const-generics/issues/issue-70125-2.rs @@ -13,4 +13,4 @@ trait Foo { } } -impl Foo<{3}> for () {} +impl Foo<3> for () {} diff --git a/src/test/ui/consts/const-block.rs b/src/test/ui/consts/const-block.rs index 7172a34c8cfeb..ec99c70f6e0b9 100644 --- a/src/test/ui/consts/const-block.rs +++ b/src/test/ui/consts/const-block.rs @@ -1,5 +1,5 @@ // run-pass - +#![allow(unused_braces)] #![allow(dead_code)] #![allow(unused_unsafe)] diff --git a/src/test/ui/expr-block-generic-unique1.rs b/src/test/ui/expr-block-generic-unique1.rs index c14191f2ffcb5..d081cb2be7ee3 100644 --- a/src/test/ui/expr-block-generic-unique1.rs +++ b/src/test/ui/expr-block-generic-unique1.rs @@ -1,5 +1,5 @@ // run-pass - +#![allow(unused_braces)] #![feature(box_syntax)] fn test_generic(expected: Box, eq: F) where T: Clone, F: FnOnce(Box, Box) -> bool { diff --git a/src/test/ui/expr-block-generic-unique2.rs b/src/test/ui/expr-block-generic-unique2.rs index 90ebc02931ad1..9362eb86fc309 100644 --- a/src/test/ui/expr-block-generic-unique2.rs +++ b/src/test/ui/expr-block-generic-unique2.rs @@ -1,5 +1,5 @@ // run-pass - +#![allow(unused_braces)] #![feature(box_syntax)] fn test_generic(expected: T, eq: F) where T: Clone, F: FnOnce(T, T) -> bool { diff --git a/src/test/ui/expr-block-generic.rs b/src/test/ui/expr-block-generic.rs index ec93f59722d0c..29c7c42219c73 100644 --- a/src/test/ui/expr-block-generic.rs +++ b/src/test/ui/expr-block-generic.rs @@ -1,4 +1,5 @@ // run-pass +#![allow(unused_braces)] fn test_generic(expected: T, eq: F) where F: FnOnce(T, T) -> bool { let actual: T = { expected.clone() }; diff --git a/src/test/ui/expr-block-unique.rs b/src/test/ui/expr-block-unique.rs index fe1a7d9f1fb31..eff3fd3a15152 100644 --- a/src/test/ui/expr-block-unique.rs +++ b/src/test/ui/expr-block-unique.rs @@ -1,5 +1,5 @@ // run-pass - +#![allow(unused_braces)] #![feature(box_syntax)] pub fn main() { let x: Box<_> = { box 100 }; assert_eq!(*x, 100); } diff --git a/src/test/ui/expr-block.rs b/src/test/ui/expr-block.rs index 549ccf9774f43..ff87595c934e9 100644 --- a/src/test/ui/expr-block.rs +++ b/src/test/ui/expr-block.rs @@ -1,10 +1,7 @@ // run-pass - +#![allow(unused_braces)] #![allow(dead_code)] - - - // Tests for standalone blocks as expressions fn test_basic() { let rs: bool = { true }; assert!((rs)); } diff --git a/src/test/ui/expr-fn.rs b/src/test/ui/expr-fn.rs index af809f563fc04..253cbfd5d38fa 100644 --- a/src/test/ui/expr-fn.rs +++ b/src/test/ui/expr-fn.rs @@ -1,4 +1,5 @@ // run-pass +#![allow(unused_braces)] fn test_int() { fn f() -> isize { 10 } diff --git a/src/test/ui/functions-closures/closure-inference.rs b/src/test/ui/functions-closures/closure-inference.rs index 96878445245bc..1877414f09942 100644 --- a/src/test/ui/functions-closures/closure-inference.rs +++ b/src/test/ui/functions-closures/closure-inference.rs @@ -1,5 +1,5 @@ // run-pass - +#![allow(unused_braces)] fn foo(i: isize) -> isize { i + 1 } diff --git a/src/test/ui/functions-closures/closure-inference2.rs b/src/test/ui/functions-closures/closure-inference2.rs index f2dfa5888aac5..4ce132e86caa4 100644 --- a/src/test/ui/functions-closures/closure-inference2.rs +++ b/src/test/ui/functions-closures/closure-inference2.rs @@ -1,6 +1,6 @@ // run-pass // Test a rather underspecified example: - +#![allow(unused_braces)] pub fn main() { let f = {|i| i}; diff --git a/src/test/ui/intrinsics/intrinsic-move-val-cleanups.rs b/src/test/ui/intrinsics/intrinsic-move-val-cleanups.rs index a2068429af5ea..9804c421db081 100644 --- a/src/test/ui/intrinsics/intrinsic-move-val-cleanups.rs +++ b/src/test/ui/intrinsics/intrinsic-move-val-cleanups.rs @@ -1,4 +1,5 @@ // run-pass +#![allow(unused_braces)] #![allow(unused_unsafe)] #![allow(unreachable_code)] // ignore-emscripten no threads support diff --git a/src/test/ui/issues/issue-23898.rs b/src/test/ui/issues/issue-23898.rs index a8787f279b71b..3de365675ad22 100644 --- a/src/test/ui/issues/issue-23898.rs +++ b/src/test/ui/issues/issue-23898.rs @@ -1,4 +1,5 @@ // run-pass +#![allow(unused_parens)] #![allow(non_camel_case_types)] // Note: This test was used to demonstrate #5873 (now #23898). diff --git a/src/test/ui/issues/issue-28777.rs b/src/test/ui/issues/issue-28777.rs index 74de00adadbda..1f426b7185e8f 100644 --- a/src/test/ui/issues/issue-28777.rs +++ b/src/test/ui/issues/issue-28777.rs @@ -1,4 +1,5 @@ // run-pass +#![allow(unused_braces)] fn main() { let v1 = { 1 + {2} * {3} }; let v2 = 1 + {2} * {3} ; diff --git a/src/test/ui/range.rs b/src/test/ui/range.rs index 82983e37ea18e..f3f7508d12434 100644 --- a/src/test/ui/range.rs +++ b/src/test/ui/range.rs @@ -1,5 +1,5 @@ // run-pass - +#![allow(unused_braces)] #![allow(unused_comparisons)] #![allow(dead_code)] #![allow(unused_mut)] diff --git a/src/test/ui/range_inclusive.rs b/src/test/ui/range_inclusive.rs index 68d9bf7d26b69..540b35e0392de 100644 --- a/src/test/ui/range_inclusive.rs +++ b/src/test/ui/range_inclusive.rs @@ -1,7 +1,7 @@ // run-pass // Test inclusive range syntax. - #![feature(range_is_empty)] +#![allow(unused_braces)] #![allow(unused_comparisons)] use std::ops::RangeToInclusive; diff --git a/src/test/ui/structs-enums/empty-tag.rs b/src/test/ui/structs-enums/empty-tag.rs index 56a438200c00b..271ab72c74fc1 100644 --- a/src/test/ui/structs-enums/empty-tag.rs +++ b/src/test/ui/structs-enums/empty-tag.rs @@ -1,4 +1,5 @@ // run-pass +#![allow(unused_braces)] #![allow(non_camel_case_types)] #[derive(Copy, Clone, Debug)] diff --git a/src/test/ui/type-alias-impl-trait/assoc-type-const.rs b/src/test/ui/type-alias-impl-trait/assoc-type-const.rs index 2907c21c6203c..7f3f86e4df009 100644 --- a/src/test/ui/type-alias-impl-trait/assoc-type-const.rs +++ b/src/test/ui/type-alias-impl-trait/assoc-type-const.rs @@ -18,16 +18,16 @@ trait MyTrait<'a, const C: usize> { const MY_CONST: usize; } -impl<'a, const C: usize> MyTrait<'a, { C }> for MyStruct<{ C }> { +impl<'a, const C: usize> MyTrait<'a, C> for MyStruct { type MyItem = u8; const MY_CONST: usize = C; } -impl<'a, I, const C: usize> UnwrapItemsExt<'a, { C }> for I { - type Iter = impl MyTrait<'a, { C }>; +impl<'a, I, const C: usize> UnwrapItemsExt<'a, C> for I { + type Iter = impl MyTrait<'a, C>; fn unwrap_items(self) -> Self::Iter { - MyStruct::<{ C }> {} + MyStruct:: {} } } diff --git a/src/test/ui/unsized-locals/unsized-exprs-rpass.rs b/src/test/ui/unsized-locals/unsized-exprs-rpass.rs index bc64fcdec2e39..24c2758a0a255 100644 --- a/src/test/ui/unsized-locals/unsized-exprs-rpass.rs +++ b/src/test/ui/unsized-locals/unsized-exprs-rpass.rs @@ -1,5 +1,5 @@ // run-pass - +#![allow(unused_braces, unused_parens)] #![feature(unsized_tuple_coercion, unsized_locals)] struct A(X); @@ -30,7 +30,6 @@ fn main() { *foo() }); udrop::<[u8]>({*foo()}); - #[allow(unused_parens)] udrop::<[u8]>((*foo())); udrop::<[u8]>((*tfoo()).1); *afoo() + 42; diff --git a/src/test/ui/weird-exprs.rs b/src/test/ui/weird-exprs.rs index ca68a5af0ddca..d812bbd011e0e 100644 --- a/src/test/ui/weird-exprs.rs +++ b/src/test/ui/weird-exprs.rs @@ -5,7 +5,7 @@ #![allow(non_camel_case_types)] #![allow(dead_code)] #![allow(unreachable_code)] -#![allow(unused_parens)] +#![allow(unused_braces, unused_parens)] #![recursion_limit = "256"] diff --git a/src/test/ui/zero-sized/zero-sized-tuple-struct.rs b/src/test/ui/zero-sized/zero-sized-tuple-struct.rs index 6c438720e5d86..2208590f7d61b 100644 --- a/src/test/ui/zero-sized/zero-sized-tuple-struct.rs +++ b/src/test/ui/zero-sized/zero-sized-tuple-struct.rs @@ -1,4 +1,5 @@ // run-pass +#![allow(unused_braces)] #![allow(unused_assignments)] // Make sure that the constructor args are codegened for zero-sized tuple structs From bcf35b1d8095625853fa6ae7fa435b662fc3241f Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Fri, 27 Mar 2020 21:56:58 +0100 Subject: [PATCH 09/12] add tests for `unused_braces` --- src/test/ui/const-generics/unused_braces.rs | 13 +++++++ .../ui/const-generics/unused_braces.stderr | 20 +++++++++++ src/test/ui/lint/unused_braces.rs | 31 ++++++++++++++++ src/test/ui/lint/unused_braces.stderr | 36 +++++++++++++++++++ src/test/ui/lint/unused_parens_borrow.rs | 22 ++++++++++++ src/test/ui/lint/unused_parens_borrow.stderr | 12 +++++++ 6 files changed, 134 insertions(+) create mode 100644 src/test/ui/const-generics/unused_braces.rs create mode 100644 src/test/ui/const-generics/unused_braces.stderr create mode 100644 src/test/ui/lint/unused_braces.rs create mode 100644 src/test/ui/lint/unused_braces.stderr create mode 100644 src/test/ui/lint/unused_parens_borrow.rs create mode 100644 src/test/ui/lint/unused_parens_borrow.stderr diff --git a/src/test/ui/const-generics/unused_braces.rs b/src/test/ui/const-generics/unused_braces.rs new file mode 100644 index 0000000000000..05234faf71420 --- /dev/null +++ b/src/test/ui/const-generics/unused_braces.rs @@ -0,0 +1,13 @@ +// check-pass +#![warn(unused_braces)] + +#![feature(const_generics)] +//~^ WARN the feature `const_generics` is incomplete and may cause the compiler to crash + +struct A; + +fn main() { + let _: A<7>; // ok + let _: A<{ 7 }>; //~ WARN unnecessary braces + let _: A<{ 3 + 5 }>; // ok +} diff --git a/src/test/ui/const-generics/unused_braces.stderr b/src/test/ui/const-generics/unused_braces.stderr new file mode 100644 index 0000000000000..fc3da6096e7d4 --- /dev/null +++ b/src/test/ui/const-generics/unused_braces.stderr @@ -0,0 +1,20 @@ +warning: the feature `const_generics` is incomplete and may cause the compiler to crash + --> $DIR/unused_braces.rs:4:12 + | +LL | #![feature(const_generics)] + | ^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + +warning: unnecessary braces around const expression + --> $DIR/unused_braces.rs:11:14 + | +LL | let _: A<{ 7 }>; + | ^^^^^ help: remove these braces + | +note: the lint level is defined here + --> $DIR/unused_braces.rs:2:9 + | +LL | #![warn(unused_braces)] + | ^^^^^^^^^^^^^ + diff --git a/src/test/ui/lint/unused_braces.rs b/src/test/ui/lint/unused_braces.rs new file mode 100644 index 0000000000000..de456ee6c230c --- /dev/null +++ b/src/test/ui/lint/unused_braces.rs @@ -0,0 +1,31 @@ +// check-pass +#![warn(unused_braces, unused_parens)] + +fn main() { + let _ = (7); + //~^WARN unnecessary parentheses + + let _ = { 7 }; + //~^ WARN unnecessary braces + + if let 7 = { 7 } { + //~^ WARN unnecessary braces + } + + let _: [u8; { 3 }]; + //~^ WARN unnecessary braces + + // do not emit error for multiline blocks. + let _ = { + 7 + }; + + // do not emit error for unsafe blocks. + let _ = unsafe { 7 }; + + // do not emit error, as the `{` would then + // be parsed as part of the `return`. + if { return } { + + } +} diff --git a/src/test/ui/lint/unused_braces.stderr b/src/test/ui/lint/unused_braces.stderr new file mode 100644 index 0000000000000..add0611eac29e --- /dev/null +++ b/src/test/ui/lint/unused_braces.stderr @@ -0,0 +1,36 @@ +warning: unnecessary parentheses around assigned value + --> $DIR/unused_braces.rs:5:13 + | +LL | let _ = (7); + | ^^^ help: remove these parentheses + | +note: the lint level is defined here + --> $DIR/unused_braces.rs:2:24 + | +LL | #![warn(unused_braces, unused_parens)] + | ^^^^^^^^^^^^^ + +warning: unnecessary braces around assigned value + --> $DIR/unused_braces.rs:8:13 + | +LL | let _ = { 7 }; + | ^^^^^ help: remove these braces + | +note: the lint level is defined here + --> $DIR/unused_braces.rs:2:9 + | +LL | #![warn(unused_braces, unused_parens)] + | ^^^^^^^^^^^^^ + +warning: unnecessary braces around `let` head expression + --> $DIR/unused_braces.rs:11:16 + | +LL | if let 7 = { 7 } { + | ^^^^^ help: remove these braces + +warning: unnecessary braces around const expression + --> $DIR/unused_braces.rs:15:17 + | +LL | let _: [u8; { 3 }]; + | ^^^^^ help: remove these braces + diff --git a/src/test/ui/lint/unused_parens_borrow.rs b/src/test/ui/lint/unused_parens_borrow.rs new file mode 100644 index 0000000000000..98dbbecfedde6 --- /dev/null +++ b/src/test/ui/lint/unused_parens_borrow.rs @@ -0,0 +1,22 @@ +// check-pass +#![warn(unused_braces)] + +// changing `&{ expr }` to `&expr` changes the semantic of the program +// so we should not warn this case + +#[repr(packed)] +struct A { + a: u8, + b: u32, +} + +fn main() { + let a = A { + a: 42, + b: 1729, + }; + + let _ = &{ a.b }; + let _ = { a.b }; + //~^ WARN unnecessary braces +} diff --git a/src/test/ui/lint/unused_parens_borrow.stderr b/src/test/ui/lint/unused_parens_borrow.stderr new file mode 100644 index 0000000000000..7e3839ae4e014 --- /dev/null +++ b/src/test/ui/lint/unused_parens_borrow.stderr @@ -0,0 +1,12 @@ +warning: unnecessary braces around assigned value + --> $DIR/unused_parens_borrow.rs:20:13 + | +LL | let _ = { a.b }; + | ^^^^^^^ help: remove these braces + | +note: the lint level is defined here + --> $DIR/unused_parens_borrow.rs:2:9 + | +LL | #![warn(unused_braces)] + | ^^^^^^^^^^^^^ + From bab327c725bf084d0ec5c27527588b71b0a1ab1d Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Tue, 31 Mar 2020 18:42:54 +0200 Subject: [PATCH 10/12] update unused_braces wording --- src/librustc_lint/unused.rs | 23 +++++++++---------- src/test/ui/lint/lint-unnecessary-parens.rs | 6 ++--- .../ui/lint/lint-unnecessary-parens.stderr | 6 ++--- src/test/ui/lint/unused_braces.stderr | 2 +- ...nused_parens_remove_json_suggestion.stderr | 4 ++-- 5 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 67e86c480a30f..51f91aa6fb792 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -327,11 +327,11 @@ enum UnusedDelimsCtx { AssignedValue, IfCond, WhileCond, - ForHeadExpr, - MatchHeadExpr, + ForIterExpr, + MatchScrutineeExpr, ReturnValue, BlockRetValue, - LetHeadExpr, + LetScrutineeExpr, ArrayLenExpr, AnonConst, } @@ -344,11 +344,11 @@ impl From for &'static str { UnusedDelimsCtx::AssignedValue => "assigned value", UnusedDelimsCtx::IfCond => "`if` condition", UnusedDelimsCtx::WhileCond => "`while` condition", - UnusedDelimsCtx::ForHeadExpr => "`for` head expression", - UnusedDelimsCtx::MatchHeadExpr => "`match` head expression", + UnusedDelimsCtx::ForIterExpr => "`for` iterator expression", + UnusedDelimsCtx::MatchScrutineeExpr => "`match` scrutinee expression", UnusedDelimsCtx::ReturnValue => "`return` value", UnusedDelimsCtx::BlockRetValue => "block return value", - UnusedDelimsCtx::LetHeadExpr => "`let` head expression", + UnusedDelimsCtx::LetScrutineeExpr => "`let` scrutinee expression", UnusedDelimsCtx::ArrayLenExpr | UnusedDelimsCtx::AnonConst => "const expression", } } @@ -399,7 +399,6 @@ trait UnusedDelimLint { self.emit_unused_delims(cx, value.span, &expr_text, ctx.into(), keep_space); } - /// emits a lint fn emit_unused_delims( &self, cx: &EarlyContext<'_>, @@ -471,12 +470,12 @@ trait UnusedDelimLint { } ForLoop(_, ref cond, ref block, ..) => { - (cond, UnusedDelimsCtx::ForHeadExpr, true, None, Some(block.span.lo())) + (cond, UnusedDelimsCtx::ForIterExpr, true, None, Some(block.span.lo())) } Match(ref head, _) => { let left = e.span.lo() + rustc_span::BytePos(5); - (head, UnusedDelimsCtx::MatchHeadExpr, true, Some(left), None) + (head, UnusedDelimsCtx::MatchScrutineeExpr, true, Some(left), None) } Ret(Some(ref value)) => { @@ -597,7 +596,7 @@ impl UnusedDelimLint for UnusedParens { self.check_unused_delims_expr( cx, expr, - UnusedDelimsCtx::LetHeadExpr, + UnusedDelimsCtx::LetScrutineeExpr, followed_by_block, None, None, @@ -732,7 +731,7 @@ impl EarlyLintPass for UnusedParens { declare_lint! { pub(super) UNUSED_BRACES, Warn, - "suggests removing `{` and `}` in case they are not necessary" + "unnecessary braces around an expression" } declare_lint_pass!(UnusedBraces => [UNUSED_BRACES]); @@ -804,7 +803,7 @@ impl UnusedDelimLint for UnusedBraces { self.check_unused_delims_expr( cx, expr, - UnusedDelimsCtx::LetHeadExpr, + UnusedDelimsCtx::LetScrutineeExpr, followed_by_block, None, None, diff --git a/src/test/ui/lint/lint-unnecessary-parens.rs b/src/test/ui/lint/lint-unnecessary-parens.rs index 5ce1f57608132..623cd04d9bce3 100644 --- a/src/test/ui/lint/lint-unnecessary-parens.rs +++ b/src/test/ui/lint/lint-unnecessary-parens.rs @@ -48,11 +48,11 @@ fn main() { if (true) {} //~ ERROR unnecessary parentheses around `if` condition while (true) {} //~ ERROR unnecessary parentheses around `while` condition //~^ WARN denote infinite loops with - match (true) { //~ ERROR unnecessary parentheses around `match` head expression + match (true) { //~ ERROR unnecessary parentheses around `match` scrutinee expression _ => {} } - if let 1 = (1) {} //~ ERROR unnecessary parentheses around `let` head expression - while let 1 = (2) {} //~ ERROR unnecessary parentheses around `let` head expression + if let 1 = (1) {} //~ ERROR unnecessary parentheses around `let` scrutinee expression + while let 1 = (2) {} //~ ERROR unnecessary parentheses around `let` scrutinee expression let v = X { y: false }; // struct lits needs parens, so these shouldn't warn. if (v == X { y: true }) {} diff --git a/src/test/ui/lint/lint-unnecessary-parens.stderr b/src/test/ui/lint/lint-unnecessary-parens.stderr index 8858c95327322..15184ba36ae85 100644 --- a/src/test/ui/lint/lint-unnecessary-parens.stderr +++ b/src/test/ui/lint/lint-unnecessary-parens.stderr @@ -72,19 +72,19 @@ LL | while (true) {} | = note: `#[warn(while_true)]` on by default -error: unnecessary parentheses around `match` head expression +error: unnecessary parentheses around `match` scrutinee expression --> $DIR/lint-unnecessary-parens.rs:51:11 | LL | match (true) { | ^^^^^^ help: remove these parentheses -error: unnecessary parentheses around `let` head expression +error: unnecessary parentheses around `let` scrutinee expression --> $DIR/lint-unnecessary-parens.rs:54:16 | LL | if let 1 = (1) {} | ^^^ help: remove these parentheses -error: unnecessary parentheses around `let` head expression +error: unnecessary parentheses around `let` scrutinee expression --> $DIR/lint-unnecessary-parens.rs:55:19 | LL | while let 1 = (2) {} diff --git a/src/test/ui/lint/unused_braces.stderr b/src/test/ui/lint/unused_braces.stderr index add0611eac29e..72f425ffc3e01 100644 --- a/src/test/ui/lint/unused_braces.stderr +++ b/src/test/ui/lint/unused_braces.stderr @@ -22,7 +22,7 @@ note: the lint level is defined here LL | #![warn(unused_braces, unused_parens)] | ^^^^^^^^^^^^^ -warning: unnecessary braces around `let` head expression +warning: unnecessary braces around `let` scrutinee expression --> $DIR/unused_braces.rs:11:16 | LL | if let 7 = { 7 } { diff --git a/src/test/ui/lint/unused_parens_remove_json_suggestion.stderr b/src/test/ui/lint/unused_parens_remove_json_suggestion.stderr index c3bf77a3a6f2d..5fb67fd7c95a3 100644 --- a/src/test/ui/lint/unused_parens_remove_json_suggestion.stderr +++ b/src/test/ui/lint/unused_parens_remove_json_suggestion.stderr @@ -46,14 +46,14 @@ LL | while(true && false) { | ^^^^^^^^^^^^^^^ help: remove these parentheses "} -{"message":"unnecessary parentheses around `for` head expression","code":{"code":"unused_parens","explanation":null},"level":"error","spans":[{"file_name":"$DIR/unused_parens_remove_json_suggestion.rs","byte_start":987,"byte_end":995,"line_start":44,"line_end":44,"column_start":18,"column_end":26,"is_primary":true,"text":[{"text":" for _ in (0 .. 3){ +{"message":"unnecessary parentheses around `for` iterator expression","code":{"code":"unused_parens","explanation":null},"level":"error","spans":[{"file_name":"$DIR/unused_parens_remove_json_suggestion.rs","byte_start":987,"byte_end":995,"line_start":44,"line_end":44,"column_start":18,"column_end":26,"is_primary":true,"text":[{"text":" for _ in (0 .. 3){ --> $DIR/unused_parens_remove_json_suggestion.rs:44:18 | LL | for _ in (0 .. 3){ | ^^^^^^^^ help: remove these parentheses "} -{"message":"unnecessary parentheses around `for` head expression","code":{"code":"unused_parens","explanation":null},"level":"error","spans":[{"file_name":"$DIR/unused_parens_remove_json_suggestion.rs","byte_start":1088,"byte_end":1096,"line_start":49,"line_end":49,"column_start":14,"column_end":22,"is_primary":true,"text":[{"text":" for _ in (0 .. 3) { +{"message":"unnecessary parentheses around `for` iterator expression","code":{"code":"unused_parens","explanation":null},"level":"error","spans":[{"file_name":"$DIR/unused_parens_remove_json_suggestion.rs","byte_start":1088,"byte_end":1096,"line_start":49,"line_end":49,"column_start":14,"column_end":22,"is_primary":true,"text":[{"text":" for _ in (0 .. 3) { --> $DIR/unused_parens_remove_json_suggestion.rs:49:14 | LL | for _ in (0 .. 3) { From 722d3d8ef06791d2ce5339606326ffcaac4fd0ca Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 31 Mar 2020 11:41:31 -0700 Subject: [PATCH 11/12] Update books. --- src/doc/book | 2 +- src/doc/nomicon | 2 +- src/doc/reference | 2 +- src/doc/rust-by-example | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/doc/book b/src/doc/book index 6fb3705e52303..c8841f2841a2d 160000 --- a/src/doc/book +++ b/src/doc/book @@ -1 +1 @@ -Subproject commit 6fb3705e5230311b096d47f7e2c91f9ce24393d0 +Subproject commit c8841f2841a2d26124319ddadd1b6a245f9a1856 diff --git a/src/doc/nomicon b/src/doc/nomicon index 9f797e65e6bcc..411197b0e7759 160000 --- a/src/doc/nomicon +++ b/src/doc/nomicon @@ -1 +1 @@ -Subproject commit 9f797e65e6bcc79419975b17aff8e21c9adc039f +Subproject commit 411197b0e77590c967e37e8f6ec681abd359afe8 diff --git a/src/doc/reference b/src/doc/reference index e2f11fe4d6a5e..89dd146154474 160000 --- a/src/doc/reference +++ b/src/doc/reference @@ -1 +1 @@ -Subproject commit e2f11fe4d6a5ecb471c70323197da43c70cb96b6 +Subproject commit 89dd146154474559536d5d4049a03831c501deea diff --git a/src/doc/rust-by-example b/src/doc/rust-by-example index cb369ae95ca36..edd2a7e687358 160000 --- a/src/doc/rust-by-example +++ b/src/doc/rust-by-example @@ -1 +1 @@ -Subproject commit cb369ae95ca36b841960182d26f6d5d9b2e3cc18 +Subproject commit edd2a7e687358712608896730c083cb76c7b401a From 4d8273dea5d2f398c473dd4a9c8a178dadc5be4c Mon Sep 17 00:00:00 2001 From: Trevor Spiteri Date: Tue, 31 Mar 2020 21:37:13 +0200 Subject: [PATCH 12/12] expand vec![] to Vec::new() --- src/liballoc/macros.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/liballoc/macros.rs b/src/liballoc/macros.rs index 422d3486f92b2..4bc0c3a079d5c 100644 --- a/src/liballoc/macros.rs +++ b/src/liballoc/macros.rs @@ -36,6 +36,9 @@ #[stable(feature = "rust1", since = "1.0.0")] #[allow_internal_unstable(box_syntax)] macro_rules! vec { + () => ( + $crate::vec::Vec::new() + ); ($elem:expr; $n:expr) => ( $crate::vec::from_elem($elem, $n) ); @@ -51,6 +54,9 @@ macro_rules! vec { // NB see the slice::hack module in slice.rs for more information #[cfg(test)] macro_rules! vec { + () => ( + $crate::vec::Vec::new() + ); ($elem:expr; $n:expr) => ( $crate::vec::from_elem($elem, $n) );