diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 84db58b8971..7d429ce4923 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -142,6 +142,7 @@ pub mod panic_unimplemented; pub mod partialeq_ne_impl; pub mod precedence; pub mod ptr; +pub mod ptr_offset_with_cast; pub mod question_mark; pub mod ranges; pub mod redundant_field_names; @@ -408,6 +409,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_late_lint_pass(box default_trait_access::DefaultTraitAccess); reg.register_late_lint_pass(box indexing_slicing::IndexingSlicing); reg.register_late_lint_pass(box non_copy_const::NonCopyConst); + reg.register_late_lint_pass(box ptr_offset_with_cast::Pass); reg.register_lint_group("clippy_restriction", vec![ arithmetic::FLOAT_ARITHMETIC, @@ -631,6 +633,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { ptr::CMP_NULL, ptr::MUT_FROM_REF, ptr::PTR_ARG, + ptr_offset_with_cast::PTR_OFFSET_WITH_CAST, question_mark::QUESTION_MARK, ranges::ITERATOR_STEP_BY_ZERO, ranges::RANGE_MINUS_ONE, @@ -815,6 +818,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL, partialeq_ne_impl::PARTIALEQ_NE_IMPL, precedence::PRECEDENCE, + ptr_offset_with_cast::PTR_OFFSET_WITH_CAST, ranges::RANGE_MINUS_ONE, ranges::RANGE_PLUS_ONE, ranges::RANGE_ZIP_WITH_LEN, diff --git a/clippy_lints/src/ptr_offset_with_cast.rs b/clippy_lints/src/ptr_offset_with_cast.rs new file mode 100644 index 00000000000..9f475473c61 --- /dev/null +++ b/clippy_lints/src/ptr_offset_with_cast.rs @@ -0,0 +1,151 @@ +use rustc::{declare_lint, hir, lint, lint_array}; +use crate::utils; +use std::fmt; + +/// **What it does:** Checks for usage of the `offset` pointer method with a `usize` casted to an +/// `isize`. +/// +/// **Why is this bad?** If we’re always increasing the pointer address, we can avoid the numeric +/// cast by using the `add` method instead. +/// +/// **Known problems:** None +/// +/// **Example:** +/// ```rust +/// let vec = vec![b'a', b'b', b'c']; +/// let ptr = vec.as_ptr(); +/// let offset = 1_usize; +/// +/// unsafe { ptr.offset(offset as isize); } +/// ``` +/// +/// Could be written: +/// +/// ```rust +/// let vec = vec![b'a', b'b', b'c']; +/// let ptr = vec.as_ptr(); +/// let offset = 1_usize; +/// +/// unsafe { ptr.add(offset); } +/// ``` +declare_clippy_lint! { + pub PTR_OFFSET_WITH_CAST, + complexity, + "uneeded pointer offset cast" +} + +#[derive(Copy, Clone, Debug)] +pub struct Pass; + +impl lint::LintPass for Pass { + fn get_lints(&self) -> lint::LintArray { + lint_array!(PTR_OFFSET_WITH_CAST) + } +} + +impl<'a, 'tcx> lint::LateLintPass<'a, 'tcx> for Pass { + fn check_expr(&mut self, cx: &lint::LateContext<'a, 'tcx>, expr: &'tcx hir::Expr) { + // Check if the expressions is a ptr.offset or ptr.wrapping_offset method call + let (receiver_expr, arg_expr, method) = match expr_as_ptr_offset_call(cx, expr) { + Some(call_arg) => call_arg, + None => return, + }; + + // Check if the argument to the method call is a cast from usize + let cast_lhs_expr = match expr_as_cast_from_usize(cx, arg_expr) { + Some(cast_lhs_expr) => cast_lhs_expr, + None => return, + }; + + let msg = format!("use of `{}` with a `usize` casted to an `isize`", method); + if let Some(sugg) = build_suggestion(cx, method, receiver_expr, cast_lhs_expr) { + utils::span_lint_and_sugg(cx, PTR_OFFSET_WITH_CAST, expr.span, &msg, "try", sugg); + } else { + utils::span_lint(cx, PTR_OFFSET_WITH_CAST, expr.span, &msg); + } + + } +} + +// If the given expression is a cast from a usize, return the lhs of the cast +fn expr_as_cast_from_usize<'a, 'tcx>( + cx: &lint::LateContext<'a, 'tcx>, + expr: &'tcx hir::Expr, +) -> Option<&'tcx hir::Expr> { + if let hir::ExprKind::Cast(ref cast_lhs_expr, _) = expr.node { + if is_expr_ty_usize(cx, &cast_lhs_expr) { + return Some(cast_lhs_expr); + } + } + None +} + +// If the given expression is a ptr::offset or ptr::wrapping_offset method call, return the +// receiver, the arg of the method call, and the method. +fn expr_as_ptr_offset_call<'a, 'tcx>( + cx: &lint::LateContext<'a, 'tcx>, + expr: &'tcx hir::Expr, +) -> Option<(&'tcx hir::Expr, &'tcx hir::Expr, Method)> { + if let hir::ExprKind::MethodCall(ref path_segment, _, ref args) = expr.node { + if is_expr_ty_raw_ptr(cx, &args[0]) { + if path_segment.ident.name == "offset" { + return Some((&args[0], &args[1], Method::Offset)); + } + if path_segment.ident.name == "wrapping_offset" { + return Some((&args[0], &args[1], Method::WrappingOffset)); + } + } + } + None +} + +// Is the type of the expression a usize? +fn is_expr_ty_usize<'a, 'tcx>( + cx: &lint::LateContext<'a, 'tcx>, + expr: &hir::Expr, +) -> bool { + cx.tables.expr_ty(expr) == cx.tcx.types.usize +} + +// Is the type of the expression a raw pointer? +fn is_expr_ty_raw_ptr<'a, 'tcx>( + cx: &lint::LateContext<'a, 'tcx>, + expr: &hir::Expr, +) -> bool { + cx.tables.expr_ty(expr).is_unsafe_ptr() +} + +fn build_suggestion<'a, 'tcx>( + cx: &lint::LateContext<'a, 'tcx>, + method: Method, + receiver_expr: &hir::Expr, + cast_lhs_expr: &hir::Expr, +) -> Option { + let receiver = utils::snippet_opt(cx, receiver_expr.span)?; + let cast_lhs = utils::snippet_opt(cx, cast_lhs_expr.span)?; + Some(format!("{}.{}({})", receiver, method.suggestion(), cast_lhs)) +} + +#[derive(Copy, Clone)] +enum Method { + Offset, + WrappingOffset, +} + +impl Method { + fn suggestion(self) -> &'static str { + match self { + Method::Offset => "add", + Method::WrappingOffset => "wrapping_add", + } + } +} + +impl fmt::Display for Method { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Method::Offset => write!(f, "offset"), + Method::WrappingOffset => write!(f, "wrapping_offset"), + } + } +} diff --git a/tests/ui/ptr_offset_with_cast.rs b/tests/ui/ptr_offset_with_cast.rs new file mode 100644 index 00000000000..4549f960ca0 --- /dev/null +++ b/tests/ui/ptr_offset_with_cast.rs @@ -0,0 +1,18 @@ +fn main() { + let vec = vec![b'a', b'b', b'c']; + let ptr = vec.as_ptr(); + + let offset_u8 = 1_u8; + let offset_usize = 1_usize; + let offset_isize = 1_isize; + + unsafe { + ptr.offset(offset_usize as isize); + ptr.offset(offset_isize as isize); + ptr.offset(offset_u8 as isize); + + ptr.wrapping_offset(offset_usize as isize); + ptr.wrapping_offset(offset_isize as isize); + ptr.wrapping_offset(offset_u8 as isize); + } +} diff --git a/tests/ui/ptr_offset_with_cast.stderr b/tests/ui/ptr_offset_with_cast.stderr new file mode 100644 index 00000000000..16bbf328b33 --- /dev/null +++ b/tests/ui/ptr_offset_with_cast.stderr @@ -0,0 +1,16 @@ +error: use of `offset` with a `usize` casted to an `isize` + --> $DIR/ptr_offset_with_cast.rs:10:9 + | +10 | ptr.offset(offset_usize as isize); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr.add(offset_usize)` + | + = note: `-D ptr-offset-with-cast` implied by `-D warnings` + +error: use of `wrapping_offset` with a `usize` casted to an `isize` + --> $DIR/ptr_offset_with_cast.rs:14:9 + | +14 | ptr.wrapping_offset(offset_usize as isize); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr.wrapping_add(offset_usize)` + +error: aborting due to 2 previous errors +