Skip to content

Commit

Permalink
New lint: Suggest ptr.add([usize]) over `ptr.offset([usize] as isiz…
Browse files Browse the repository at this point in the history
…e)`.

First part of #3047.
  • Loading branch information
frewsxcv committed Aug 29, 2018
1 parent 8901272 commit e0c7eb7
Show file tree
Hide file tree
Showing 4 changed files with 158 additions and 0 deletions.
4 changes: 4 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -755,6 +758,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
panic_unimplemented::PANIC_PARAMS,
ptr::CMP_NULL,
ptr::PTR_ARG,
ptr_offset_with_cast::PTR_OFFSET_WITH_CAST,
question_mark::QUESTION_MARK,
redundant_field_names::REDUNDANT_FIELD_NAMES,
regex::REGEX_MACRO,
Expand Down
130 changes: 130 additions & 0 deletions clippy_lints/src/ptr_offset_with_cast.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
use rustc::{declare_lint, hir, lint, lint_array, ty};
use syntax::ast;
use crate::utils;

/// **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,
style,
"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 method call
let [receiver_expr, arg_expr] = match expr_as_ptr_offset_call(cx, expr) {
Some(call_arg) => call_arg,
None => return,
};

// Check if the parameter to ptr.offset is a cast from usize to isize
let cast_lhs_expr = match expr_as_cast_from_usize(cx, arg_expr) {
Some(cast_lhs_expr) => cast_lhs_expr,
None => return,
};

utils::span_lint_and_sugg(
cx,
PTR_OFFSET_WITH_CAST,
expr.span,
"use of `offset` with a `usize` casted to an `isize`",
"try",
build_suggestion(cx, receiver_expr, cast_lhs_expr),
);
}
}

// 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 method call, return the receiver and the arg of the
// method call.
fn expr_as_ptr_offset_call<'a, 'tcx>(
cx: &lint::LateContext<'a, 'tcx>,
expr: &'tcx hir::Expr,
) -> Option<[&'tcx hir::Expr; 2]> {
if let hir::ExprKind::MethodCall(ref path_segment, _, ref args) = expr.node {
if path_segment.ident.name == "offset" && is_expr_ty_raw_ptr(cx, &args[0]) {
return Some([&args[0], &args[1]]);
}
}
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).sty == ty::TyKind::Uint(ast::UintTy::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 {
if let ty::RawPtr(..) = cx.tables.expr_ty(expr).sty {
true
} else {
false
}
}

fn build_suggestion<'a, 'tcx>(
cx: &lint::LateContext<'a, 'tcx>,
receiver_expr: &hir::Expr,
cast_lhs_expr: &hir::Expr,
) -> String {
match (
utils::snippet_opt(cx, receiver_expr.span),
utils::snippet_opt(cx, cast_lhs_expr.span)
) {
(Some(receiver), Some(cast_lhs)) => format!("{}.add({})", receiver, cast_lhs),
_ => String::new(),
}
}
14 changes: 14 additions & 0 deletions tests/ui/ptr_offset_with_cast.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
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);
}
}
10 changes: 10 additions & 0 deletions tests/ui/ptr_offset_with_cast.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
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: aborting due to previous error

0 comments on commit e0c7eb7

Please sign in to comment.