Skip to content
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7404,6 +7404,7 @@ Released 2018-09-13
[`vec_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#vec_box
[`vec_init_then_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#vec_init_then_push
[`vec_resize_to_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#vec_resize_to_zero
[`vec_to_rc_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#vec_to_rc_slice
Comment thread
landaire marked this conversation as resolved.
[`verbose_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_bit_mask
[`verbose_file_reads`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_file_reads
[`volatile_composites`]: https://rust-lang.github.io/rust-clippy/master/index.html#volatile_composites
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -794,6 +794,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
crate::useless_conversion::USELESS_CONVERSION_INFO,
crate::useless_vec::USELESS_VEC_INFO,
crate::vec_init_then_push::VEC_INIT_THEN_PUSH_INFO,
crate::vec_to_rc_slice::VEC_TO_RC_SLICE_INFO,
crate::visibility::NEEDLESS_PUB_SELF_INFO,
crate::visibility::PUB_WITH_SHORTHAND_INFO,
crate::visibility::PUB_WITHOUT_SHORTHAND_INFO,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,7 @@ mod useless_concat;
mod useless_conversion;
mod useless_vec;
mod vec_init_then_push;
mod vec_to_rc_slice;
mod visibility;
mod volatile_composites;
mod wildcard_imports;
Expand Down Expand Up @@ -867,6 +868,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
Box::new(move |tcx| Box::new(manual_pop_if::ManualPopIf::new(tcx, conf))),
Box::new(move |_| Box::new(manual_noop_waker::ManualNoopWaker::new(conf))),
Box::new(|_| Box::new(byte_char_slices::ByteCharSlice)),
Box::new(|_| Box::new(vec_to_rc_slice::VecToRcSlice)),
// add late passes here, used by `cargo dev new_lint`
];
store.late_passes.extend(late_lints);
Expand Down
157 changes: 157 additions & 0 deletions clippy_lints/src/vec_to_rc_slice.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::res::{MaybeDef, MaybeResPath, MaybeTypeckRes};
use clippy_utils::source::snippet_with_context;
use clippy_utils::{qpath_generic_tys, sym};
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, Node, TyKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{self, Ty};
use rustc_session::declare_lint_pass;
use rustc_span::Span;

declare_clippy_lint! {
/// ### What it does
/// Detects conversions from `Vec<T>` to `Arc<[T]>` or `Rc<[T]>` via `.into()`,
/// `Arc::from()`, `Rc::from()`, or `From::from()`.
///
/// ### Why is this bad?
/// `Arc<[T]>` and `Rc<[T]>` store the reference count and slice data in a single
/// contiguous allocation. Because `Vec<T>` uses a separate heap allocation with a
/// different layout, converting `Vec<T>` into `Arc<[T]>` or `Rc<[T]>` must allocate
/// a new block and **copy** all elements. For large vectors this copy can be expensive.
///
/// Using `Arc<Box<[T]>>` (or `Rc<Box<[T]>>`) avoids the copy:
/// `Vec::into_boxed_slice()` can reuse the existing allocation (shrinking if needed),
/// and wrapping the resulting `Box<[T]>` in an `Arc`/`Rc` is a cheap pointer-sized
/// allocation.
///
/// The trade-off is one extra level of indirection when accessing the data.
///
/// ### Example
/// ```no_run
/// # use std::sync::Arc;
/// let v: Vec<u8> = vec![1, 2, 3];
/// let a: Arc<[u8]> = v.into();
/// ```
/// Use instead:
/// ```no_run
/// # use std::sync::Arc;
/// let v: Vec<u8> = vec![1, 2, 3];
/// let a: Arc<Box<[u8]>> = Arc::new(v.into_boxed_slice());
/// ```
#[clippy::version = "1.97.0"]
pub VEC_TO_RC_SLICE,
perf,
"converting `Vec<T>` to `Arc<[T]>` or `Rc<[T]>` copies all elements to a new allocation"
}

declare_lint_pass!(VecToRcSlice => [VEC_TO_RC_SLICE]);

/// Checks if `ty` is `Vec<T>` for some `T`.
fn is_vec(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
if let ty::Adt(adt, _) = ty.kind() {
cx.tcx.is_diagnostic_item(sym::Vec, adt.did())
} else {
false
}
}

/// If `ty` is `Arc<[T]>` or `Rc<[T]>`, returns the wrapper name (`"Arc"` or `"Rc"`).
fn rc_slice_wrapper(cx: &LateContext<'_>, ty: Ty<'_>) -> Option<&'static str> {
if let ty::Adt(adt, args) = ty.kind()
&& let Some(inner) = args.types().next()
&& inner.is_slice()
{
match cx.tcx.get_diagnostic_name(adt.did()) {
Some(sym::Arc) => Some("Arc"),
Some(sym::Rc) => Some("Rc"),
_ => None,
}
} else {
None
}
}

/// If the expression is the init of a `let` statement with a type annotation like
/// `Arc<[T]>` or `Rc<[T]>`, returns the span of the inner slice type (e.g. `[T]`).
fn let_ty_inner_slice_span(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<Span> {
if let Node::LetStmt(let_stmt) = cx.tcx.parent_hir_node(expr.hir_id)
&& let Some(hir_ty) = let_stmt.ty
&& let TyKind::Path(ref qpath) = hir_ty.kind
&& let Some(def_id) = hir_ty.basic_res().opt_def_id()
&& matches!(cx.tcx.get_diagnostic_name(def_id), Some(sym::Arc | sym::Rc))
&& let Some(inner_ty) = qpath_generic_tys(qpath).next()
&& matches!(inner_ty.kind, TyKind::Slice(_))
{
Some(inner_ty.span)
} else {
None
}
}

impl<'tcx> LateLintPass<'tcx> for VecToRcSlice {
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
if e.span.from_expansion() {
return;
}

match e.kind {
// `vec_expr.into()`
ExprKind::MethodCall(name, recv, [], _) => {
if name.ident.name == sym::into
&& cx.ty_based_def(e).opt_parent(cx).is_diag_item(cx, sym::Into)
&& is_vec(cx, cx.typeck_results().expr_ty(recv))
&& let Some(wrapper) = rc_slice_wrapper(cx, cx.typeck_results().expr_ty(e))
{
emit_lint(cx, e, recv, wrapper);
}
},

// `Arc::from(vec)` / `Rc::from(vec)` / `From::from(vec)`
ExprKind::Call(path, [arg]) => {
if let ExprKind::Path(ref qpath) = path.kind
&& let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id()
&& cx.tcx.is_diagnostic_item(sym::from_fn, def_id)
&& is_vec(cx, cx.typeck_results().expr_ty(arg))
&& let Some(wrapper) = rc_slice_wrapper(cx, cx.typeck_results().expr_ty(e))
{
emit_lint(cx, e, arg, wrapper);
}
},

_ => {},
}
}
}

fn emit_lint(cx: &LateContext<'_>, expr: &Expr<'_>, vec_expr: &Expr<'_>, wrapper: &str) {
span_lint_and_then(
cx,
VEC_TO_RC_SLICE,
expr.span,
format!("converting a `Vec<T>` to `{wrapper}<[T]>` copies all elements to a new allocation"),
|diag| {
let mut app = Applicability::MaybeIncorrect;
let ctxt = expr.span.ctxt();

// Common case: we have a `Vec` that we can call `into_boxed_slice()` on. Generate the snippet
// which does so with the appropriate span to identify the Vec and constructs the target (wrapper)
// type from it too.
//
// e.g. `Rc::from(vec)` -> `Rc::new(vec.into_boxed_slice())`
let vec_snippet = snippet_with_context(cx, vec_expr.span, ctxt, "<vec>", &mut app).0;
let expr_sugg = format!("{wrapper}::new({vec_snippet}.into_boxed_slice())");
let mut sugg = vec![(expr.span, expr_sugg)];

// if `expr` is part of a let stmt with a type ascription, also fix the latter as part of the
// suggestion.
let inner_slice_span = let_ty_inner_slice_span(cx, expr);
if let Some(ty_span) = inner_slice_span {
let slice_snippet = snippet_with_context(cx, ty_span, ctxt, "_", &mut app).0;
sugg.push((ty_span, format!("Box<{slice_snippet}>")));
}
Comment thread
ada4a marked this conversation as resolved.

diag.multipart_suggestion(format!("convert target type to `{wrapper}<Box<[T]>>`"), sugg, app);
},
);
}
65 changes: 65 additions & 0 deletions tests/ui/vec_to_rc_slice.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#![warn(clippy::vec_to_rc_slice)]

use std::rc::Rc;
use std::sync::Arc;

fn main() {
// Should lint: Vec<T>.into() -> Arc<[T]>
let v: Vec<u8> = vec![1, 2, 3];
let _a: Arc<Box<[u8]>> = Arc::new(v.into_boxed_slice());
//~^ vec_to_rc_slice

// Should lint: Arc::from(vec)
let v: Vec<u8> = vec![1, 2, 3];
let _a: Arc<Box<[u8]>> = Arc::new(v.into_boxed_slice());
//~^ vec_to_rc_slice

// Should lint: From::from(vec) when target is Arc<[T]>
let v: Vec<u8> = vec![1, 2, 3];
let _a: Arc<Box<[u8]>> = Arc::new(v.into_boxed_slice());
//~^ vec_to_rc_slice

// Should lint: fully qualified Arc from
let v: Vec<u8> = vec![1, 2, 3];
let _a = Arc::new(v.into_boxed_slice());
//~^ vec_to_rc_slice

// Should lint: Vec<T>.into() -> Rc<[T]>
let v: Vec<u8> = vec![1, 2, 3];
let _a: Rc<Box<[u8]>> = Rc::new(v.into_boxed_slice());
//~^ vec_to_rc_slice

// Should lint: Rc::from(vec)
let v: Vec<u8> = vec![1, 2, 3];
let _a: Rc<Box<[u8]>> = Rc::new(v.into_boxed_slice());
//~^ vec_to_rc_slice

// Should lint: From::from(vec) when target is Rc<[T]>
let v: Vec<u8> = vec![1, 2, 3];
let _a: Rc<Box<[u8]>> = Rc::new(v.into_boxed_slice());
//~^ vec_to_rc_slice

// Should lint: fully qualified Rc from
let v: Vec<u8> = vec![1, 2, 3];
let _a = Rc::new(v.into_boxed_slice());
//~^ vec_to_rc_slice

// Should NOT lint: Vec<T>.into() -> something else
let v: Vec<u8> = vec![1, 2, 3];
let _b: Box<[u8]> = v.into();

// Should NOT lint: non-Vec into Arc<[T]>
let _c: Arc<[u8]> = Arc::from([1u8, 2, 3].as_slice());

// Should NOT lint: Vec into Arc (not Arc<[T]>)
let v: Vec<u8> = vec![1, 2, 3];
let _d: Arc<Vec<u8>> = Arc::new(v);

// Should NOT lint: the recommended pattern
let v: Vec<u8> = vec![1, 2, 3];
let _e: Arc<Box<[u8]>> = Arc::new(v.into_boxed_slice());

// Should NOT lint: Rc recommended pattern
let v: Vec<u8> = vec![1, 2, 3];
let _f: Rc<Box<[u8]>> = Rc::new(v.into_boxed_slice());
}
65 changes: 65 additions & 0 deletions tests/ui/vec_to_rc_slice.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#![warn(clippy::vec_to_rc_slice)]

use std::rc::Rc;
use std::sync::Arc;

fn main() {
// Should lint: Vec<T>.into() -> Arc<[T]>
let v: Vec<u8> = vec![1, 2, 3];
let _a: Arc<[u8]> = v.into();
//~^ vec_to_rc_slice

// Should lint: Arc::from(vec)
let v: Vec<u8> = vec![1, 2, 3];
let _a: Arc<[u8]> = Arc::from(v);
//~^ vec_to_rc_slice

// Should lint: From::from(vec) when target is Arc<[T]>
let v: Vec<u8> = vec![1, 2, 3];
let _a: Arc<[u8]> = From::from(v);
//~^ vec_to_rc_slice

// Should lint: fully qualified Arc from
let v: Vec<u8> = vec![1, 2, 3];
let _a = <Arc<[u8]> as From<Vec<u8>>>::from(v);
//~^ vec_to_rc_slice

// Should lint: Vec<T>.into() -> Rc<[T]>
let v: Vec<u8> = vec![1, 2, 3];
let _a: Rc<[u8]> = v.into();
//~^ vec_to_rc_slice

// Should lint: Rc::from(vec)
let v: Vec<u8> = vec![1, 2, 3];
let _a: Rc<[u8]> = Rc::from(v);
//~^ vec_to_rc_slice

// Should lint: From::from(vec) when target is Rc<[T]>
let v: Vec<u8> = vec![1, 2, 3];
let _a: Rc<[u8]> = From::from(v);
//~^ vec_to_rc_slice

// Should lint: fully qualified Rc from
let v: Vec<u8> = vec![1, 2, 3];
let _a = <Rc<[u8]> as From<Vec<u8>>>::from(v);
//~^ vec_to_rc_slice

// Should NOT lint: Vec<T>.into() -> something else
let v: Vec<u8> = vec![1, 2, 3];
let _b: Box<[u8]> = v.into();

// Should NOT lint: non-Vec into Arc<[T]>
let _c: Arc<[u8]> = Arc::from([1u8, 2, 3].as_slice());

// Should NOT lint: Vec into Arc (not Arc<[T]>)
let v: Vec<u8> = vec![1, 2, 3];
let _d: Arc<Vec<u8>> = Arc::new(v);

// Should NOT lint: the recommended pattern
let v: Vec<u8> = vec![1, 2, 3];
let _e: Arc<Box<[u8]>> = Arc::new(v.into_boxed_slice());

// Should NOT lint: Rc recommended pattern
let v: Vec<u8> = vec![1, 2, 3];
let _f: Rc<Box<[u8]>> = Rc::new(v.into_boxed_slice());
}
Loading
Loading