diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index b0ea9689e50d6..731b5f77dd326 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -122,6 +122,7 @@ declare_lint_pass! { UNSAFE_OP_IN_UNSAFE_FN, UNSTABLE_NAME_COLLISIONS, UNSTABLE_SYNTAX_PRE_EXPANSION, + UNSUPPORTED_CALLING_CONVENTIONS, UNSUPPORTED_FN_PTR_CALLING_CONVENTIONS, UNUSED_ASSIGNMENTS, UNUSED_ASSOCIATED_TYPE_BOUNDS, diff --git a/library/std/src/sys/pal/hermit/thread.rs b/library/std/src/sys/pal/hermit/thread.rs index bb68a824fc313..0437a3c93e9bc 100644 --- a/library/std/src/sys/pal/hermit/thread.rs +++ b/library/std/src/sys/pal/hermit/thread.rs @@ -58,7 +58,11 @@ impl Thread { } } - pub unsafe fn new(stack: usize, p: Box) -> io::Result { + pub unsafe fn new( + stack: usize, + _name: Option<&str>, + p: Box, + ) -> io::Result { unsafe { Thread::new_with_coreid(stack, p, -1 /* = no specific core */) } diff --git a/library/std/src/sys/pal/itron/thread.rs b/library/std/src/sys/pal/itron/thread.rs index a974f4f17ae67..c6cea9ab0103d 100644 --- a/library/std/src/sys/pal/itron/thread.rs +++ b/library/std/src/sys/pal/itron/thread.rs @@ -86,7 +86,11 @@ impl Thread { /// # Safety /// /// See `thread::Builder::spawn_unchecked` for safety requirements. - pub unsafe fn new(stack: usize, p: Box) -> io::Result { + pub unsafe fn new( + stack: usize, + _name: Option<&str>, + p: Box, + ) -> io::Result { let inner = Box::new(ThreadInner { start: UnsafeCell::new(ManuallyDrop::new(p)), lifecycle: AtomicUsize::new(LIFECYCLE_INIT), diff --git a/library/std/src/sys/pal/sgx/thread.rs b/library/std/src/sys/pal/sgx/thread.rs index 219ef1b7a9897..a0b0d06596250 100644 --- a/library/std/src/sys/pal/sgx/thread.rs +++ b/library/std/src/sys/pal/sgx/thread.rs @@ -96,7 +96,11 @@ pub mod wait_notify { impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements - pub unsafe fn new(_stack: usize, p: Box) -> io::Result { + pub unsafe fn new( + _stack: usize, + _name: Option<&str>, + p: Box, + ) -> io::Result { let mut queue_lock = task_queue::lock(); unsafe { usercalls::launch_thread()? }; let (task, handle) = task_queue::Task::new(p); diff --git a/library/std/src/sys/pal/teeos/thread.rs b/library/std/src/sys/pal/teeos/thread.rs index e3b4908f85863..a5b26ee961abf 100644 --- a/library/std/src/sys/pal/teeos/thread.rs +++ b/library/std/src/sys/pal/teeos/thread.rs @@ -22,7 +22,11 @@ unsafe extern "C" { impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements - pub unsafe fn new(stack: usize, p: Box) -> io::Result { + pub unsafe fn new( + stack: usize, + _name: Option<&str>, + p: Box, + ) -> io::Result { let p = Box::into_raw(Box::new(p)); let mut native: libc::pthread_t = unsafe { mem::zeroed() }; let mut attr: libc::pthread_attr_t = unsafe { mem::zeroed() }; diff --git a/library/std/src/sys/pal/uefi/thread.rs b/library/std/src/sys/pal/uefi/thread.rs index 7d4006ff4b2f7..e1e66eaca1ba3 100644 --- a/library/std/src/sys/pal/uefi/thread.rs +++ b/library/std/src/sys/pal/uefi/thread.rs @@ -11,7 +11,11 @@ pub const DEFAULT_MIN_STACK_SIZE: usize = 64 * 1024; impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements - pub unsafe fn new(_stack: usize, _p: Box) -> io::Result { + pub unsafe fn new( + _stack: usize, + _name: Option<&str>, + _p: Box, + ) -> io::Result { unsupported() } diff --git a/library/std/src/sys/pal/unix/stack_overflow.rs b/library/std/src/sys/pal/unix/stack_overflow.rs index a3be2cdf738f5..d89100e69196a 100644 --- a/library/std/src/sys/pal/unix/stack_overflow.rs +++ b/library/std/src/sys/pal/unix/stack_overflow.rs @@ -8,8 +8,8 @@ pub struct Handler { } impl Handler { - pub unsafe fn new() -> Handler { - make_handler(false) + pub unsafe fn new(thread_name: Option>) -> Handler { + make_handler(false, thread_name) } fn null() -> Handler { @@ -72,7 +72,6 @@ mod imp { use crate::sync::OnceLock; use crate::sync::atomic::{Atomic, AtomicBool, AtomicPtr, AtomicUsize, Ordering}; use crate::sys::pal::unix::os; - use crate::thread::with_current_name; use crate::{io, mem, panic, ptr}; // Signal handler for the SIGSEGV and SIGBUS handlers. We've got guard pages @@ -158,13 +157,12 @@ mod imp { if !NEED_ALTSTACK.load(Ordering::Relaxed) { // haven't set up our sigaltstack yet NEED_ALTSTACK.store(true, Ordering::Release); - let handler = unsafe { make_handler(true) }; + let handler = unsafe { make_handler(true, None) }; MAIN_ALTSTACK.store(handler.data, Ordering::Relaxed); mem::forget(handler); if let Some(guard_page_range) = guard_page_range.take() { - let thread_name = with_current_name(|name| name.map(Box::from)); - set_current_info(guard_page_range, thread_name); + set_current_info(guard_page_range, Some(Box::from("main"))); } } @@ -230,14 +228,13 @@ mod imp { /// # Safety /// Mutates the alternate signal stack #[forbid(unsafe_op_in_unsafe_fn)] - pub unsafe fn make_handler(main_thread: bool) -> Handler { + pub unsafe fn make_handler(main_thread: bool, thread_name: Option>) -> Handler { if !NEED_ALTSTACK.load(Ordering::Acquire) { return Handler::null(); } if !main_thread { if let Some(guard_page_range) = unsafe { current_guard() } { - let thread_name = with_current_name(|name| name.map(Box::from)); set_current_info(guard_page_range, thread_name); } } @@ -634,7 +631,10 @@ mod imp { pub unsafe fn cleanup() {} - pub unsafe fn make_handler(_main_thread: bool) -> super::Handler { + pub unsafe fn make_handler( + _main_thread: bool, + _thread_name: Option>, + ) -> super::Handler { super::Handler::null() } @@ -717,7 +717,10 @@ mod imp { pub unsafe fn cleanup() {} - pub unsafe fn make_handler(main_thread: bool) -> super::Handler { + pub unsafe fn make_handler( + main_thread: bool, + _thread_name: Option>, + ) -> super::Handler { if !main_thread { reserve_stack(); } diff --git a/library/std/src/sys/pal/unix/thread.rs b/library/std/src/sys/pal/unix/thread.rs index d8b189413f4a3..5f2b81b0588a9 100644 --- a/library/std/src/sys/pal/unix/thread.rs +++ b/library/std/src/sys/pal/unix/thread.rs @@ -22,6 +22,11 @@ pub const DEFAULT_MIN_STACK_SIZE: usize = 256 * 1024; #[cfg(any(target_os = "espidf", target_os = "nuttx"))] pub const DEFAULT_MIN_STACK_SIZE: usize = 0; // 0 indicates that the stack size configured in the ESP-IDF/NuttX menuconfig system should be used +struct ThreadData { + name: Option>, + f: Box, +} + pub struct Thread { id: libc::pthread_t, } @@ -34,8 +39,12 @@ unsafe impl Sync for Thread {} impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces - pub unsafe fn new(stack: usize, p: Box) -> io::Result { - let p = Box::into_raw(Box::new(p)); + pub unsafe fn new( + stack: usize, + name: Option<&str>, + f: Box, + ) -> io::Result { + let data = Box::into_raw(Box::new(ThreadData { name: name.map(Box::from), f })); let mut native: libc::pthread_t = mem::zeroed(); let mut attr: mem::MaybeUninit = mem::MaybeUninit::uninit(); assert_eq!(libc::pthread_attr_init(attr.as_mut_ptr()), 0); @@ -73,7 +82,7 @@ impl Thread { }; } - let ret = libc::pthread_create(&mut native, attr.as_ptr(), thread_start, p as *mut _); + let ret = libc::pthread_create(&mut native, attr.as_ptr(), thread_start, data as *mut _); // Note: if the thread creation fails and this assert fails, then p will // be leaked. However, an alternative design could cause double-free // which is clearly worse. @@ -82,19 +91,20 @@ impl Thread { return if ret != 0 { // The thread failed to start and as a result p was not consumed. Therefore, it is // safe to reconstruct the box so that it gets deallocated. - drop(Box::from_raw(p)); + drop(Box::from_raw(data)); Err(io::Error::from_raw_os_error(ret)) } else { Ok(Thread { id: native }) }; - extern "C" fn thread_start(main: *mut libc::c_void) -> *mut libc::c_void { + extern "C" fn thread_start(data: *mut libc::c_void) -> *mut libc::c_void { unsafe { + let data = Box::from_raw(data as *mut ThreadData); // Next, set up our stack overflow handler which may get triggered if we run // out of stack. - let _handler = stack_overflow::Handler::new(); + let _handler = stack_overflow::Handler::new(data.name); // Finally, let's run some code. - Box::from_raw(main as *mut Box)(); + (data.f)(); } ptr::null_mut() } diff --git a/library/std/src/sys/pal/unsupported/thread.rs b/library/std/src/sys/pal/unsupported/thread.rs index 89f8bad7026ee..51b4916fcda78 100644 --- a/library/std/src/sys/pal/unsupported/thread.rs +++ b/library/std/src/sys/pal/unsupported/thread.rs @@ -10,7 +10,11 @@ pub const DEFAULT_MIN_STACK_SIZE: usize = 64 * 1024; impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements - pub unsafe fn new(_stack: usize, _p: Box) -> io::Result { + pub unsafe fn new( + _stack: usize, + _name: Option<&str>, + _p: Box, + ) -> io::Result { unsupported() } diff --git a/library/std/src/sys/pal/wasi/thread.rs b/library/std/src/sys/pal/wasi/thread.rs index cc569bb3daf68..52abd7b8f78ed 100644 --- a/library/std/src/sys/pal/wasi/thread.rs +++ b/library/std/src/sys/pal/wasi/thread.rs @@ -73,7 +73,7 @@ impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements cfg_if::cfg_if! { if #[cfg(target_feature = "atomics")] { - pub unsafe fn new(stack: usize, p: Box) -> io::Result { + pub unsafe fn new(stack: usize, _name: Option<&str>, p: Box) -> io::Result { let p = Box::into_raw(Box::new(p)); let mut native: libc::pthread_t = unsafe { mem::zeroed() }; let mut attr: libc::pthread_attr_t = unsafe { mem::zeroed() }; @@ -120,7 +120,7 @@ impl Thread { } } } else { - pub unsafe fn new(_stack: usize, _p: Box) -> io::Result { + pub unsafe fn new(_stack: usize, _name: Option<&str>, _p: Box) -> io::Result { crate::sys::unsupported() } } diff --git a/library/std/src/sys/pal/wasm/atomics/thread.rs b/library/std/src/sys/pal/wasm/atomics/thread.rs index dd5aff391fd8b..5d5c40f55685e 100644 --- a/library/std/src/sys/pal/wasm/atomics/thread.rs +++ b/library/std/src/sys/pal/wasm/atomics/thread.rs @@ -10,7 +10,11 @@ pub const DEFAULT_MIN_STACK_SIZE: usize = 1024 * 1024; impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements - pub unsafe fn new(_stack: usize, _p: Box) -> io::Result { + pub unsafe fn new( + _stack: usize, + _name: Option<&str>, + _p: Box, + ) -> io::Result { unsupported() } diff --git a/library/std/src/sys/pal/windows/thread.rs b/library/std/src/sys/pal/windows/thread.rs index 45e52cf4d047f..66891ea10b2a2 100644 --- a/library/std/src/sys/pal/windows/thread.rs +++ b/library/std/src/sys/pal/windows/thread.rs @@ -20,7 +20,11 @@ pub struct Thread { impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces - pub unsafe fn new(stack: usize, p: Box) -> io::Result { + pub unsafe fn new( + stack: usize, + _name: Option<&str>, + p: Box, + ) -> io::Result { let p = Box::into_raw(Box::new(p)); // CreateThread rounds up values for the stack size to the nearest page size (at least 4kb). diff --git a/library/std/src/sys/pal/xous/thread.rs b/library/std/src/sys/pal/xous/thread.rs index 0ebb46dc19faa..a2a93e5ecad33 100644 --- a/library/std/src/sys/pal/xous/thread.rs +++ b/library/std/src/sys/pal/xous/thread.rs @@ -20,7 +20,11 @@ pub const GUARD_PAGE_SIZE: usize = 4096; impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements - pub unsafe fn new(stack: usize, p: Box) -> io::Result { + pub unsafe fn new( + stack: usize, + _name: Option<&str>, + p: Box, + ) -> io::Result { let p = Box::into_raw(Box::new(p)); let mut stack_size = crate::cmp::max(stack, MIN_STACK_SIZE); diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index 26b2fb4472436..491e3ac1d46f4 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -595,7 +595,7 @@ impl Builder { // Similarly, the `sys` implementation must guarantee that no references to the closure // exist after the thread has terminated, which is signaled by `Thread::join` // returning. - native: unsafe { imp::Thread::new(stack_size, main)? }, + native: unsafe { imp::Thread::new(stack_size, my_thread.name(), main)? }, thread: my_thread, packet: my_packet, }) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 0aedc7f5219c8..567ceec6dc332 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -647,7 +647,20 @@ impl Item { ) -> hir::FnHeader { let sig = tcx.fn_sig(def_id).skip_binder(); let constness = if tcx.is_const_fn(def_id) { - hir::Constness::Const + // rustc's `is_const_fn` returns `true` for associated functions that have an `impl const` parent + // or that have a `#[const_trait]` parent. Do not display those as `const` in rustdoc because we + // won't be printing correct syntax plus the syntax is unstable. + match tcx.opt_associated_item(def_id) { + Some(ty::AssocItem { + container: ty::AssocItemContainer::Impl, + trait_item_def_id: Some(_), + .. + }) + | Some(ty::AssocItem { container: ty::AssocItemContainer::Trait, .. }) => { + hir::Constness::NotConst + } + None | Some(_) => hir::Constness::Const, + } } else { hir::Constness::NotConst }; diff --git a/src/tools/clippy/clippy_lints/src/format_args.rs b/src/tools/clippy/clippy_lints/src/format_args.rs index 0c39aae9ca913..fd1e6700698ce 100644 --- a/src/tools/clippy/clippy_lints/src/format_args.rs +++ b/src/tools/clippy/clippy_lints/src/format_args.rs @@ -163,7 +163,7 @@ declare_clippy_lint! { /// nothing will be suggested, e.g. `println!("{0}={1}", var, 1+2)`. #[clippy::version = "1.66.0"] pub UNINLINED_FORMAT_ARGS, - style, + pedantic, "using non-inlined variables in `format!` calls" } diff --git a/src/tools/clippy/clippy_lints/src/methods/manual_is_variant_and.rs b/src/tools/clippy/clippy_lints/src/methods/manual_is_variant_and.rs index 4a61c223d2c1e..40aad03960c43 100644 --- a/src/tools/clippy/clippy_lints/src/methods/manual_is_variant_and.rs +++ b/src/tools/clippy/clippy_lints/src/methods/manual_is_variant_and.rs @@ -1,22 +1,18 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::get_parent_expr; use clippy_utils::msrvs::{self, Msrv}; -use clippy_utils::source::{snippet, snippet_opt}; +use clippy_utils::source::snippet; use clippy_utils::ty::is_type_diagnostic_item; use rustc_errors::Applicability; -use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res}; -use rustc_hir::{BinOpKind, Expr, ExprKind, QPath}; use rustc_lint::LateContext; -use rustc_middle::ty; -use rustc_span::{BytePos, Span, sym}; +use rustc_span::{Span, sym}; use super::MANUAL_IS_VARIANT_AND; -pub(super) fn check( +pub(super) fn check<'tcx>( cx: &LateContext<'_>, - expr: &Expr<'_>, - map_recv: &Expr<'_>, - map_arg: &Expr<'_>, + expr: &'tcx rustc_hir::Expr<'_>, + map_recv: &'tcx rustc_hir::Expr<'_>, + map_arg: &'tcx rustc_hir::Expr<'_>, map_span: Span, msrv: Msrv, ) { @@ -61,57 +57,3 @@ pub(super) fn check( Applicability::MachineApplicable, ); } - -fn emit_lint(cx: &LateContext<'_>, op: BinOpKind, parent: &Expr<'_>, method_span: Span, is_option: bool) { - if let Some(before_map_snippet) = snippet_opt(cx, parent.span.with_hi(method_span.lo())) - && let Some(after_map_snippet) = snippet_opt(cx, method_span.with_lo(method_span.lo() + BytePos(3))) - { - span_lint_and_sugg( - cx, - MANUAL_IS_VARIANT_AND, - parent.span, - format!( - "called `.map() {}= {}()`", - if op == BinOpKind::Eq { '=' } else { '!' }, - if is_option { "Some" } else { "Ok" }, - ), - "use", - if is_option && op == BinOpKind::Ne { - format!("{before_map_snippet}is_none_or{after_map_snippet}",) - } else { - format!( - "{}{before_map_snippet}{}{after_map_snippet}", - if op == BinOpKind::Eq { "" } else { "!" }, - if is_option { "is_some_and" } else { "is_ok_and" }, - ) - }, - Applicability::MachineApplicable, - ); - } -} - -pub(super) fn check_map(cx: &LateContext<'_>, expr: &Expr<'_>) { - if let Some(parent_expr) = get_parent_expr(cx, expr) - && let ExprKind::Binary(op, left, right) = parent_expr.kind - && matches!(op.node, BinOpKind::Eq | BinOpKind::Ne) - && op.span.eq_ctxt(expr.span) - { - // Check `left` and `right` expression in any order, and for `Option` and `Result` - for (expr1, expr2) in [(left, right), (right, left)] { - for item in [sym::Option, sym::Result] { - if let ExprKind::Call(call, ..) = expr1.kind - && let ExprKind::Path(QPath::Resolved(_, path)) = call.kind - && let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Fn), _) = path.res - && let ty = cx.typeck_results().expr_ty(expr1) - && let ty::Adt(adt, args) = ty.kind() - && cx.tcx.is_diagnostic_item(item, adt.did()) - && args.type_at(0).is_bool() - && let ExprKind::MethodCall(_, recv, _, span) = expr2.kind - && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), item) - { - return emit_lint(cx, op.node, parent_expr, span, item == sym::Option); - } - } - } - } -} diff --git a/src/tools/clippy/clippy_lints/src/methods/mod.rs b/src/tools/clippy/clippy_lints/src/methods/mod.rs index 347960e0003d7..6d3f59520a264 100644 --- a/src/tools/clippy/clippy_lints/src/methods/mod.rs +++ b/src/tools/clippy/clippy_lints/src/methods/mod.rs @@ -5242,7 +5242,6 @@ impl Methods { unused_enumerate_index::check(cx, expr, recv, m_arg); map_clone::check(cx, expr, recv, m_arg, self.msrv); map_with_unused_argument_over_ranges::check(cx, expr, recv, m_arg, self.msrv, span); - manual_is_variant_and::check_map(cx, expr); match method_call(recv) { Some((map_name @ (sym::iter | sym::into_iter), recv2, _, _, _)) => { iter_kv_map::check(cx, map_name, expr, recv2, m_arg, self.msrv); diff --git a/src/tools/clippy/clippy_lints/src/methods/return_and_then.rs b/src/tools/clippy/clippy_lints/src/methods/return_and_then.rs index df8544f92203e..91643b0dfefde 100644 --- a/src/tools/clippy/clippy_lints/src/methods/return_and_then.rs +++ b/src/tools/clippy/clippy_lints/src/methods/return_and_then.rs @@ -9,7 +9,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::{indent_of, reindent_multiline, snippet_with_applicability}; use clippy_utils::ty::get_type_diagnostic_name; use clippy_utils::visitors::for_each_unconsumed_temporary; -use clippy_utils::{get_parent_expr, peel_blocks}; +use clippy_utils::{is_expr_final_block_expr, peel_blocks}; use super::RETURN_AND_THEN; @@ -21,7 +21,7 @@ pub(super) fn check<'tcx>( recv: &'tcx hir::Expr<'tcx>, arg: &'tcx hir::Expr<'_>, ) { - if cx.tcx.hir_get_fn_id_for_return_block(expr.hir_id).is_none() { + if !is_expr_final_block_expr(cx.tcx, expr) { return; } @@ -55,24 +55,12 @@ pub(super) fn check<'tcx>( None => &body_snip, }; - // If suggestion is going to get inserted as part of a `return` expression, it must be blockified. - let sugg = if let Some(parent_expr) = get_parent_expr(cx, expr) { - let base_indent = indent_of(cx, parent_expr.span); - let inner_indent = base_indent.map(|i| i + 4); - format!( - "{}\n{}\n{}", - reindent_multiline(&format!("{{\nlet {arg_snip} = {recv_snip}?;"), true, inner_indent), - reindent_multiline(inner, false, inner_indent), - reindent_multiline("}", false, base_indent), - ) - } else { - format!( - "let {} = {}?;\n{}", - arg_snip, - recv_snip, - reindent_multiline(inner, false, indent_of(cx, expr.span)) - ) - }; + let sugg = format!( + "let {} = {}?;\n{}", + arg_snip, + recv_snip, + reindent_multiline(inner, false, indent_of(cx, expr.span)) + ); span_lint_and_sugg( cx, diff --git a/src/tools/clippy/clippy_lints/src/methods/swap_with_temporary.rs b/src/tools/clippy/clippy_lints/src/methods/swap_with_temporary.rs index de729fb343a34..e378cbd6ae0ad 100644 --- a/src/tools/clippy/clippy_lints/src/methods/swap_with_temporary.rs +++ b/src/tools/clippy/clippy_lints/src/methods/swap_with_temporary.rs @@ -4,6 +4,7 @@ use rustc_ast::BorrowKind; use rustc_errors::{Applicability, Diag}; use rustc_hir::{Expr, ExprKind, Node, QPath}; use rustc_lint::LateContext; +use rustc_middle::ty::adjustment::Adjust; use rustc_span::sym; use super::SWAP_WITH_TEMPORARY; @@ -11,12 +12,12 @@ use super::SWAP_WITH_TEMPORARY; const MSG_TEMPORARY: &str = "this expression returns a temporary value"; const MSG_TEMPORARY_REFMUT: &str = "this is a mutable reference to a temporary value"; -pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, func: &Expr<'_>, args: &[Expr<'_>]) { +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, func: &Expr<'_>, args: &'tcx [Expr<'_>]) { if let ExprKind::Path(QPath::Resolved(_, func_path)) = func.kind && let Some(func_def_id) = func_path.res.opt_def_id() && cx.tcx.is_diagnostic_item(sym::mem_swap, func_def_id) { - match (ArgKind::new(&args[0]), ArgKind::new(&args[1])) { + match (ArgKind::new(cx, &args[0]), ArgKind::new(cx, &args[1])) { (ArgKind::RefMutToTemp(left_temp), ArgKind::RefMutToTemp(right_temp)) => { emit_lint_useless(cx, expr, &args[0], &args[1], left_temp, right_temp); }, @@ -28,10 +29,10 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, func: &Expr<'_>, args } enum ArgKind<'tcx> { - // Mutable reference to a place, coming from a macro - RefMutToPlaceAsMacro(&'tcx Expr<'tcx>), - // Place behind a mutable reference - RefMutToPlace(&'tcx Expr<'tcx>), + // Mutable reference to a place, coming from a macro, and number of dereferences to use + RefMutToPlaceAsMacro(&'tcx Expr<'tcx>, usize), + // Place behind a mutable reference, and number of dereferences to use + RefMutToPlace(&'tcx Expr<'tcx>, usize), // Temporary value behind a mutable reference RefMutToTemp(&'tcx Expr<'tcx>), // Any other case @@ -39,13 +40,29 @@ enum ArgKind<'tcx> { } impl<'tcx> ArgKind<'tcx> { - fn new(arg: &'tcx Expr<'tcx>) -> Self { - if let ExprKind::AddrOf(BorrowKind::Ref, _, target) = arg.kind { - if target.is_syntactic_place_expr() { + /// Build a new `ArgKind` from `arg`. There must be no false positive when returning a + /// `ArgKind::RefMutToTemp` variant, as this may cause a spurious lint to be emitted. + fn new(cx: &LateContext<'tcx>, arg: &'tcx Expr<'tcx>) -> Self { + if let ExprKind::AddrOf(BorrowKind::Ref, _, target) = arg.kind + && let adjustments = cx.typeck_results().expr_adjustments(arg) + && adjustments + .first() + .is_some_and(|adj| matches!(adj.kind, Adjust::Deref(None))) + && adjustments + .last() + .is_some_and(|adj| matches!(adj.kind, Adjust::Borrow(_))) + { + let extra_derefs = adjustments[1..adjustments.len() - 1] + .iter() + .filter(|adj| matches!(adj.kind, Adjust::Deref(_))) + .count(); + // If a deref is used, `arg` might be a place expression. For example, a mutex guard + // would dereference into the mutex content which is probably not temporary. + if target.is_syntactic_place_expr() || extra_derefs > 0 { if arg.span.from_expansion() { - ArgKind::RefMutToPlaceAsMacro(arg) + ArgKind::RefMutToPlaceAsMacro(arg, extra_derefs) } else { - ArgKind::RefMutToPlace(target) + ArgKind::RefMutToPlace(target, extra_derefs) } } else { ArgKind::RefMutToTemp(target) @@ -106,10 +123,15 @@ fn emit_lint_assign(cx: &LateContext<'_>, expr: &Expr<'_>, target: &ArgKind<'_>, let mut applicability = Applicability::MachineApplicable; let ctxt = expr.span.ctxt(); let assign_target = match target { - ArgKind::Expr(target) | ArgKind::RefMutToPlaceAsMacro(target) => { - Sugg::hir_with_context(cx, target, ctxt, "_", &mut applicability).deref() - }, - ArgKind::RefMutToPlace(target) => Sugg::hir_with_context(cx, target, ctxt, "_", &mut applicability), + ArgKind::Expr(target) => Sugg::hir_with_context(cx, target, ctxt, "_", &mut applicability).deref(), + ArgKind::RefMutToPlaceAsMacro(arg, derefs) => (0..*derefs).fold( + Sugg::hir_with_context(cx, arg, ctxt, "_", &mut applicability).deref(), + |sugg, _| sugg.deref(), + ), + ArgKind::RefMutToPlace(target, derefs) => (0..*derefs).fold( + Sugg::hir_with_context(cx, target, ctxt, "_", &mut applicability), + |sugg, _| sugg.deref(), + ), ArgKind::RefMutToTemp(_) => unreachable!(), }; let assign_source = Sugg::hir_with_context(cx, temp, ctxt, "_", &mut applicability); diff --git a/src/tools/clippy/tests/ui/manual_is_variant_and.fixed b/src/tools/clippy/tests/ui/manual_is_variant_and.fixed index 18a72188ab593..c9c184561dd69 100644 --- a/src/tools/clippy/tests/ui/manual_is_variant_and.fixed +++ b/src/tools/clippy/tests/ui/manual_is_variant_and.fixed @@ -4,44 +4,6 @@ #[macro_use] extern crate option_helpers; -struct Foo(T); - -impl Foo { - fn map bool>(self, mut f: F) -> Option { - Some(f(self.0)) - } -} - -fn foo() -> Option { - Some(true) -} - -macro_rules! some_true { - () => { - Some(true) - }; -} -macro_rules! some_false { - () => { - Some(false) - }; -} - -macro_rules! mac { - (some $e:expr) => { - Some($e) - }; - (some_map $e:expr) => { - Some($e).map(|x| x % 2 == 0) - }; - (map $e:expr) => { - $e.map(|x| x % 2 == 0) - }; - (eq $a:expr, $b:expr) => { - $a == $b - }; -} - #[rustfmt::skip] fn option_methods() { let opt = Some(1); @@ -59,15 +21,6 @@ fn option_methods() { let _ = opt .is_some_and(|x| x > 1); - let _ = Some(2).is_some_and(|x| x % 2 == 0); - //~^ manual_is_variant_and - let _ = Some(2).is_none_or(|x| x % 2 == 0); - //~^ manual_is_variant_and - let _ = Some(2).is_some_and(|x| x % 2 == 0); - //~^ manual_is_variant_and - let _ = Some(2).is_none_or(|x| x % 2 == 0); - //~^ manual_is_variant_and - // won't fix because the return type of the closure is not `bool` let _ = opt.map(|x| x + 1).unwrap_or_default(); @@ -75,14 +28,6 @@ fn option_methods() { let _ = opt2.is_some_and(char::is_alphanumeric); // should lint //~^ manual_is_variant_and let _ = opt_map!(opt2, |x| x == 'a').unwrap_or_default(); // should not lint - - // Should not lint. - let _ = Foo::(0).map(|x| x % 2 == 0) == Some(true); - let _ = Some(2).map(|x| x % 2 == 0) != foo(); - let _ = mac!(eq Some(2).map(|x| x % 2 == 0), Some(true)); - let _ = mac!(some 2).map(|x| x % 2 == 0) == Some(true); - let _ = mac!(some_map 2) == Some(true); - let _ = mac!(map Some(2)) == Some(true); } #[rustfmt::skip] @@ -96,13 +41,6 @@ fn result_methods() { }); let _ = res.is_ok_and(|x| x > 1); - let _ = Ok::(2).is_ok_and(|x| x % 2 == 0); - //~^ manual_is_variant_and - let _ = !Ok::(2).is_ok_and(|x| x % 2 == 0); - //~^ manual_is_variant_and - let _ = !Ok::(2).is_ok_and(|x| x % 2 == 0); - //~^ manual_is_variant_and - // won't fix because the return type of the closure is not `bool` let _ = res.map(|x| x + 1).unwrap_or_default(); diff --git a/src/tools/clippy/tests/ui/manual_is_variant_and.rs b/src/tools/clippy/tests/ui/manual_is_variant_and.rs index a92f7c0436959..52c7b56804ce2 100644 --- a/src/tools/clippy/tests/ui/manual_is_variant_and.rs +++ b/src/tools/clippy/tests/ui/manual_is_variant_and.rs @@ -4,44 +4,6 @@ #[macro_use] extern crate option_helpers; -struct Foo(T); - -impl Foo { - fn map bool>(self, mut f: F) -> Option { - Some(f(self.0)) - } -} - -fn foo() -> Option { - Some(true) -} - -macro_rules! some_true { - () => { - Some(true) - }; -} -macro_rules! some_false { - () => { - Some(false) - }; -} - -macro_rules! mac { - (some $e:expr) => { - Some($e) - }; - (some_map $e:expr) => { - Some($e).map(|x| x % 2 == 0) - }; - (map $e:expr) => { - $e.map(|x| x % 2 == 0) - }; - (eq $a:expr, $b:expr) => { - $a == $b - }; -} - #[rustfmt::skip] fn option_methods() { let opt = Some(1); @@ -65,15 +27,6 @@ fn option_methods() { //~^ manual_is_variant_and .unwrap_or_default(); - let _ = Some(2).map(|x| x % 2 == 0) == Some(true); - //~^ manual_is_variant_and - let _ = Some(2).map(|x| x % 2 == 0) != Some(true); - //~^ manual_is_variant_and - let _ = Some(2).map(|x| x % 2 == 0) == some_true!(); - //~^ manual_is_variant_and - let _ = Some(2).map(|x| x % 2 == 0) != some_false!(); - //~^ manual_is_variant_and - // won't fix because the return type of the closure is not `bool` let _ = opt.map(|x| x + 1).unwrap_or_default(); @@ -81,14 +34,6 @@ fn option_methods() { let _ = opt2.map(char::is_alphanumeric).unwrap_or_default(); // should lint //~^ manual_is_variant_and let _ = opt_map!(opt2, |x| x == 'a').unwrap_or_default(); // should not lint - - // Should not lint. - let _ = Foo::(0).map(|x| x % 2 == 0) == Some(true); - let _ = Some(2).map(|x| x % 2 == 0) != foo(); - let _ = mac!(eq Some(2).map(|x| x % 2 == 0), Some(true)); - let _ = mac!(some 2).map(|x| x % 2 == 0) == Some(true); - let _ = mac!(some_map 2) == Some(true); - let _ = mac!(map Some(2)) == Some(true); } #[rustfmt::skip] @@ -105,13 +50,6 @@ fn result_methods() { //~^ manual_is_variant_and .unwrap_or_default(); - let _ = Ok::(2).map(|x| x % 2 == 0) == Ok(true); - //~^ manual_is_variant_and - let _ = Ok::(2).map(|x| x % 2 == 0) != Ok(true); - //~^ manual_is_variant_and - let _ = Ok::(2).map(|x| x % 2 == 0) != Ok(true); - //~^ manual_is_variant_and - // won't fix because the return type of the closure is not `bool` let _ = res.map(|x| x + 1).unwrap_or_default(); diff --git a/src/tools/clippy/tests/ui/manual_is_variant_and.stderr b/src/tools/clippy/tests/ui/manual_is_variant_and.stderr index 1fb437a8bc744..a4fa500580d0a 100644 --- a/src/tools/clippy/tests/ui/manual_is_variant_and.stderr +++ b/src/tools/clippy/tests/ui/manual_is_variant_and.stderr @@ -1,5 +1,5 @@ error: called `map().unwrap_or_default()` on an `Option` value - --> tests/ui/manual_is_variant_and.rs:51:17 + --> tests/ui/manual_is_variant_and.rs:13:17 | LL | let _ = opt.map(|x| x > 1) | _________________^ @@ -11,7 +11,7 @@ LL | | .unwrap_or_default(); = help: to override `-D warnings` add `#[allow(clippy::manual_is_variant_and)]` error: called `map().unwrap_or_default()` on an `Option` value - --> tests/ui/manual_is_variant_and.rs:56:17 + --> tests/ui/manual_is_variant_and.rs:18:17 | LL | let _ = opt.map(|x| { | _________________^ @@ -30,13 +30,13 @@ LL ~ }); | error: called `map().unwrap_or_default()` on an `Option` value - --> tests/ui/manual_is_variant_and.rs:61:17 + --> tests/ui/manual_is_variant_and.rs:23:17 | LL | let _ = opt.map(|x| x > 1).unwrap_or_default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_some_and(|x| x > 1)` error: called `map().unwrap_or_default()` on an `Option` value - --> tests/ui/manual_is_variant_and.rs:64:10 + --> tests/ui/manual_is_variant_and.rs:26:10 | LL | .map(|x| x > 1) | __________^ @@ -44,38 +44,14 @@ LL | | LL | | .unwrap_or_default(); | |____________________________^ help: use: `is_some_and(|x| x > 1)` -error: called `.map() == Some()` - --> tests/ui/manual_is_variant_and.rs:68:13 - | -LL | let _ = Some(2).map(|x| x % 2 == 0) == Some(true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_some_and(|x| x % 2 == 0)` - -error: called `.map() != Some()` - --> tests/ui/manual_is_variant_and.rs:70:13 - | -LL | let _ = Some(2).map(|x| x % 2 == 0) != Some(true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_none_or(|x| x % 2 == 0)` - -error: called `.map() == Some()` - --> tests/ui/manual_is_variant_and.rs:72:13 - | -LL | let _ = Some(2).map(|x| x % 2 == 0) == some_true!(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_some_and(|x| x % 2 == 0)` - -error: called `.map() != Some()` - --> tests/ui/manual_is_variant_and.rs:74:13 - | -LL | let _ = Some(2).map(|x| x % 2 == 0) != some_false!(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_none_or(|x| x % 2 == 0)` - error: called `map().unwrap_or_default()` on an `Option` value - --> tests/ui/manual_is_variant_and.rs:81:18 + --> tests/ui/manual_is_variant_and.rs:34:18 | LL | let _ = opt2.map(char::is_alphanumeric).unwrap_or_default(); // should lint | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_some_and(char::is_alphanumeric)` error: called `map().unwrap_or_default()` on a `Result` value - --> tests/ui/manual_is_variant_and.rs:99:17 + --> tests/ui/manual_is_variant_and.rs:44:17 | LL | let _ = res.map(|x| { | _________________^ @@ -94,7 +70,7 @@ LL ~ }); | error: called `map().unwrap_or_default()` on a `Result` value - --> tests/ui/manual_is_variant_and.rs:104:17 + --> tests/ui/manual_is_variant_and.rs:49:17 | LL | let _ = res.map(|x| x > 1) | _________________^ @@ -102,29 +78,11 @@ LL | | LL | | .unwrap_or_default(); | |____________________________^ help: use: `is_ok_and(|x| x > 1)` -error: called `.map() == Ok()` - --> tests/ui/manual_is_variant_and.rs:108:13 - | -LL | let _ = Ok::(2).map(|x| x % 2 == 0) == Ok(true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Ok::(2).is_ok_and(|x| x % 2 == 0)` - -error: called `.map() != Ok()` - --> tests/ui/manual_is_variant_and.rs:110:13 - | -LL | let _ = Ok::(2).map(|x| x % 2 == 0) != Ok(true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `!Ok::(2).is_ok_and(|x| x % 2 == 0)` - -error: called `.map() != Ok()` - --> tests/ui/manual_is_variant_and.rs:112:13 - | -LL | let _ = Ok::(2).map(|x| x % 2 == 0) != Ok(true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `!Ok::(2).is_ok_and(|x| x % 2 == 0)` - error: called `map().unwrap_or_default()` on a `Result` value - --> tests/ui/manual_is_variant_and.rs:119:18 + --> tests/ui/manual_is_variant_and.rs:57:18 | LL | let _ = res2.map(char::is_alphanumeric).unwrap_or_default(); // should lint | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_ok_and(char::is_alphanumeric)` -error: aborting due to 15 previous errors +error: aborting due to 8 previous errors diff --git a/src/tools/clippy/tests/ui/return_and_then.fixed b/src/tools/clippy/tests/ui/return_and_then.fixed index 8d9481d159512..74efa14eeec81 100644 --- a/src/tools/clippy/tests/ui/return_and_then.fixed +++ b/src/tools/clippy/tests/ui/return_and_then.fixed @@ -67,60 +67,8 @@ fn main() { .first() // creates temporary reference .and_then(|x| test_opt_block(Some(*x))) } - - fn in_closure() -> bool { - let _ = || { - let x = Some("")?; - if x.len() > 2 { Some(3) } else { None } - //~^ return_and_then - }; - true - } - - fn with_return(shortcut: bool) -> Option { - if shortcut { - return { - let x = Some("")?; - if x.len() > 2 { Some(3) } else { None } - }; - //~^ return_and_then - }; - None - } - - fn with_return_multiline(shortcut: bool) -> Option { - if shortcut { - return { - let mut x = Some("")?; - let x = format!("{x}."); - if x.len() > 2 { Some(3) } else { None } - }; - //~^^^^ return_and_then - }; - None - } } fn gen_option(n: i32) -> Option { Some(n) } - -mod issue14781 { - fn foo(_: &str, _: (u32, u32)) -> Result<(u32, u32), ()> { - Ok((1, 1)) - } - - fn bug(_: Option<&str>) -> Result<(), ()> { - let year: Option<&str> = None; - let month: Option<&str> = None; - let day: Option<&str> = None; - - let _day = if let (Some(year), Some(month)) = (year, month) { - day.and_then(|day| foo(day, (1, 31)).ok()) - } else { - None - }; - - Ok(()) - } -} diff --git a/src/tools/clippy/tests/ui/return_and_then.rs b/src/tools/clippy/tests/ui/return_and_then.rs index beada921a9187..188dc57e588cf 100644 --- a/src/tools/clippy/tests/ui/return_and_then.rs +++ b/src/tools/clippy/tests/ui/return_and_then.rs @@ -63,55 +63,8 @@ fn main() { .first() // creates temporary reference .and_then(|x| test_opt_block(Some(*x))) } - - fn in_closure() -> bool { - let _ = || { - Some("").and_then(|x| if x.len() > 2 { Some(3) } else { None }) - //~^ return_and_then - }; - true - } - - fn with_return(shortcut: bool) -> Option { - if shortcut { - return Some("").and_then(|x| if x.len() > 2 { Some(3) } else { None }); - //~^ return_and_then - }; - None - } - - fn with_return_multiline(shortcut: bool) -> Option { - if shortcut { - return Some("").and_then(|mut x| { - let x = format!("{x}."); - if x.len() > 2 { Some(3) } else { None } - }); - //~^^^^ return_and_then - }; - None - } } fn gen_option(n: i32) -> Option { Some(n) } - -mod issue14781 { - fn foo(_: &str, _: (u32, u32)) -> Result<(u32, u32), ()> { - Ok((1, 1)) - } - - fn bug(_: Option<&str>) -> Result<(), ()> { - let year: Option<&str> = None; - let month: Option<&str> = None; - let day: Option<&str> = None; - - let _day = if let (Some(year), Some(month)) = (year, month) { - day.and_then(|day| foo(day, (1, 31)).ok()) - } else { - None - }; - - Ok(()) - } -} diff --git a/src/tools/clippy/tests/ui/return_and_then.stderr b/src/tools/clippy/tests/ui/return_and_then.stderr index 5feca88286057..a7acbe7b3401c 100644 --- a/src/tools/clippy/tests/ui/return_and_then.stderr +++ b/src/tools/clippy/tests/ui/return_and_then.stderr @@ -101,50 +101,5 @@ LL + })?; LL + if x.len() > 2 { Some(3) } else { None } | -error: use the `?` operator instead of an `and_then` call - --> tests/ui/return_and_then.rs:69:13 - | -LL | Some("").and_then(|x| if x.len() > 2 { Some(3) } else { None }) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: try - | -LL ~ let x = Some("")?; -LL + if x.len() > 2 { Some(3) } else { None } - | - -error: use the `?` operator instead of an `and_then` call - --> tests/ui/return_and_then.rs:77:20 - | -LL | return Some("").and_then(|x| if x.len() > 2 { Some(3) } else { None }); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: try - | -LL ~ return { -LL + let x = Some("")?; -LL + if x.len() > 2 { Some(3) } else { None } -LL ~ }; - | - -error: use the `?` operator instead of an `and_then` call - --> tests/ui/return_and_then.rs:85:20 - | -LL | return Some("").and_then(|mut x| { - | ____________________^ -LL | | let x = format!("{x}."); -LL | | if x.len() > 2 { Some(3) } else { None } -LL | | }); - | |______________^ - | -help: try - | -LL ~ return { -LL + let mut x = Some("")?; -LL + let x = format!("{x}."); -LL + if x.len() > 2 { Some(3) } else { None } -LL ~ }; - | - -error: aborting due to 10 previous errors +error: aborting due to 7 previous errors diff --git a/src/tools/clippy/tests/ui/swap_with_temporary.fixed b/src/tools/clippy/tests/ui/swap_with_temporary.fixed index 4007d998ba068..24c667dd47958 100644 --- a/src/tools/clippy/tests/ui/swap_with_temporary.fixed +++ b/src/tools/clippy/tests/ui/swap_with_temporary.fixed @@ -72,3 +72,49 @@ fn dont_lint_those(s: &mut S, v: &mut [String], w: Option<&mut String>) { swap(&mut s.t, v.get_mut(0).unwrap()); swap(w.unwrap(), &mut s.t); } + +fn issue15166() { + use std::sync::Mutex; + + struct A { + thing: Mutex>, + } + + impl A { + fn a(&self) { + let mut new_vec = vec![42]; + // Do not lint here, as neither `new_vec` nor the result of `.lock().unwrap()` are temporaries + swap(&mut new_vec, &mut self.thing.lock().unwrap()); + for v in new_vec { + // Do something with v + } + // Here `vec![42]` is temporary though, and a proper dereference will have to be used in the fix + *self.thing.lock().unwrap() = vec![42]; + //~^ ERROR: swapping with a temporary value is inefficient + } + } +} + +fn multiple_deref() { + let mut v1 = &mut &mut &mut vec![42]; + ***v1 = vec![]; + //~^ ERROR: swapping with a temporary value is inefficient + + struct Wrapper(T); + impl std::ops::Deref for Wrapper { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } + } + impl std::ops::DerefMut for Wrapper { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } + } + + use std::sync::Mutex; + let mut v1 = Mutex::new(Wrapper(Wrapper(vec![42]))); + *(*(*v1.lock().unwrap())) = vec![]; + //~^ ERROR: swapping with a temporary value is inefficient +} diff --git a/src/tools/clippy/tests/ui/swap_with_temporary.rs b/src/tools/clippy/tests/ui/swap_with_temporary.rs index d403c086c0f4f..8e35e6144d99a 100644 --- a/src/tools/clippy/tests/ui/swap_with_temporary.rs +++ b/src/tools/clippy/tests/ui/swap_with_temporary.rs @@ -72,3 +72,49 @@ fn dont_lint_those(s: &mut S, v: &mut [String], w: Option<&mut String>) { swap(&mut s.t, v.get_mut(0).unwrap()); swap(w.unwrap(), &mut s.t); } + +fn issue15166() { + use std::sync::Mutex; + + struct A { + thing: Mutex>, + } + + impl A { + fn a(&self) { + let mut new_vec = vec![42]; + // Do not lint here, as neither `new_vec` nor the result of `.lock().unwrap()` are temporaries + swap(&mut new_vec, &mut self.thing.lock().unwrap()); + for v in new_vec { + // Do something with v + } + // Here `vec![42]` is temporary though, and a proper dereference will have to be used in the fix + swap(&mut vec![42], &mut self.thing.lock().unwrap()); + //~^ ERROR: swapping with a temporary value is inefficient + } + } +} + +fn multiple_deref() { + let mut v1 = &mut &mut &mut vec![42]; + swap(&mut ***v1, &mut vec![]); + //~^ ERROR: swapping with a temporary value is inefficient + + struct Wrapper(T); + impl std::ops::Deref for Wrapper { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } + } + impl std::ops::DerefMut for Wrapper { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } + } + + use std::sync::Mutex; + let mut v1 = Mutex::new(Wrapper(Wrapper(vec![42]))); + swap(&mut vec![], &mut v1.lock().unwrap()); + //~^ ERROR: swapping with a temporary value is inefficient +} diff --git a/src/tools/clippy/tests/ui/swap_with_temporary.stderr b/src/tools/clippy/tests/ui/swap_with_temporary.stderr index 59355771a9648..c0ea592fb05a7 100644 --- a/src/tools/clippy/tests/ui/swap_with_temporary.stderr +++ b/src/tools/clippy/tests/ui/swap_with_temporary.stderr @@ -96,5 +96,41 @@ note: this expression returns a temporary value LL | swap(mac!(refmut y), &mut func()); | ^^^^^^ -error: aborting due to 8 previous errors +error: swapping with a temporary value is inefficient + --> tests/ui/swap_with_temporary.rs:92:13 + | +LL | swap(&mut vec![42], &mut self.thing.lock().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use assignment instead: `*self.thing.lock().unwrap() = vec![42]` + | +note: this expression returns a temporary value + --> tests/ui/swap_with_temporary.rs:92:23 + | +LL | swap(&mut vec![42], &mut self.thing.lock().unwrap()); + | ^^^^^^^^ + +error: swapping with a temporary value is inefficient + --> tests/ui/swap_with_temporary.rs:100:5 + | +LL | swap(&mut ***v1, &mut vec![]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use assignment instead: `***v1 = vec![]` + | +note: this expression returns a temporary value + --> tests/ui/swap_with_temporary.rs:100:27 + | +LL | swap(&mut ***v1, &mut vec![]); + | ^^^^^^ + +error: swapping with a temporary value is inefficient + --> tests/ui/swap_with_temporary.rs:118:5 + | +LL | swap(&mut vec![], &mut v1.lock().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use assignment instead: `*(*(*v1.lock().unwrap())) = vec![]` + | +note: this expression returns a temporary value + --> tests/ui/swap_with_temporary.rs:118:15 + | +LL | swap(&mut vec![], &mut v1.lock().unwrap()); + | ^^^^^^ + +error: aborting due to 11 previous errors diff --git a/tests/rustdoc/constant/const-trait-and-impl-methods.rs b/tests/rustdoc/constant/const-trait-and-impl-methods.rs new file mode 100644 index 0000000000000..30fc539e5533f --- /dev/null +++ b/tests/rustdoc/constant/const-trait-and-impl-methods.rs @@ -0,0 +1,36 @@ +// check that we don't render `#[const_trait]` methods as `const` - even for +// const `trait`s and `impl`s. +#![crate_name = "foo"] +#![feature(const_trait_impl)] + +//@ has foo/trait.Tr.html +//@ has - '//*[@id="tymethod.required"]' 'fn required()' +//@ !has - '//*[@id="tymethod.required"]' 'const' +//@ has - '//*[@id="method.defaulted"]' 'fn defaulted()' +//@ !has - '//*[@id="method.defaulted"]' 'const' +#[const_trait] +pub trait Tr { + fn required(); + fn defaulted() {} +} + +pub struct ConstImpl {} +pub struct NonConstImpl {} + +//@ has foo/struct.ConstImpl.html +//@ has - '//*[@id="method.required"]' 'fn required()' +//@ !has - '//*[@id="method.required"]' 'const' +//@ has - '//*[@id="method.defaulted"]' 'fn defaulted()' +//@ !has - '//*[@id="method.defaulted"]' 'const' +impl const Tr for ConstImpl { + fn required() {} +} + +//@ has foo/struct.NonConstImpl.html +//@ has - '//*[@id="method.required"]' 'fn required()' +//@ !has - '//*[@id="method.required"]' 'const' +//@ has - '//*[@id="method.defaulted"]' 'fn defaulted()' +//@ !has - '//*[@id="method.defaulted"]' 'const' +impl Tr for NonConstImpl { + fn required() {} +} diff --git a/tests/ui/runtime/out-of-stack.rs b/tests/ui/runtime/out-of-stack.rs index 6be34afb56035..913d3637c8f2b 100644 --- a/tests/ui/runtime/out-of-stack.rs +++ b/tests/ui/runtime/out-of-stack.rs @@ -19,7 +19,7 @@ extern crate libc; use std::env; use std::hint::black_box; use std::process::Command; -use std::thread; +use std::thread::Builder; fn silent_recurse() { let buf = [0u8; 1000]; @@ -56,9 +56,9 @@ fn main() { } else if args.len() > 1 && args[1] == "loud" { loud_recurse(); } else if args.len() > 1 && args[1] == "silent-thread" { - thread::spawn(silent_recurse).join(); + Builder::new().name("ferris".to_string()).spawn(silent_recurse).unwrap().join(); } else if args.len() > 1 && args[1] == "loud-thread" { - thread::spawn(loud_recurse).join(); + Builder::new().name("ferris".to_string()).spawn(loud_recurse).unwrap().join(); } else { let mut modes = vec![ "silent-thread", @@ -82,6 +82,12 @@ fn main() { let error = String::from_utf8_lossy(&silent.stderr); assert!(error.contains("has overflowed its stack"), "missing overflow message: {}", error); + + if mode.contains("thread") { + assert!(error.contains("ferris"), "missing thread name: {}", error); + } else { + assert!(error.contains("main"), "missing thread name: {}", error); + } } } }