Skip to content

Commit

Permalink
Auto merge of #117758 - Urgau:lint_pointer_trait_comparisons, r=david…
Browse files Browse the repository at this point in the history
…twco

Add lint against ambiguous wide pointer comparisons

This PR is the resolution of #106447 decided in #117717 by T-lang.

## `ambiguous_wide_pointer_comparisons`

*warn-by-default*

The `ambiguous_wide_pointer_comparisons` lint checks comparison of `*const/*mut ?Sized` as the operands.

### Example

```rust
let ab = (A, B);
let a = &ab.0 as *const dyn T;
let b = &ab.1 as *const dyn T;

let _ = a == b;
```

### Explanation

The comparison includes metadata which may not be expected.

-------

This PR also drops `clippy::vtable_address_comparisons` which is superseded by this one.

~~One thing: is the current naming right? `invalid` seems a bit too much.~~

Fixes #117717
  • Loading branch information
bors committed Dec 11, 2023
2 parents 1f9b674 + a2ea760 commit c8213a4
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 244 deletions.
1 change: 0 additions & 1 deletion clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::unit_types::UNIT_ARG_INFO,
crate::unit_types::UNIT_CMP_INFO,
crate::unnamed_address::FN_ADDRESS_COMPARISONS_INFO,
crate::unnamed_address::VTABLE_ADDRESS_COMPARISONS_INFO,
crate::unnecessary_box_returns::UNNECESSARY_BOX_RETURNS_INFO,
crate::unnecessary_map_on_constructor::UNNECESSARY_MAP_ON_CONSTRUCTOR_INFO,
crate::unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS_INFO,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/renamed_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,5 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[
("clippy::undropped_manually_drops", "undropped_manually_drops"),
("clippy::unknown_clippy_lints", "unknown_lints"),
("clippy::unused_label", "unused_labels"),
("clippy::vtable_address_comparisons", "ambiguous_wide_pointer_comparisons"),
];
68 changes: 2 additions & 66 deletions clippy_lints/src/unnamed_address.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
use clippy_utils::diagnostics::span_lint;
use rustc_hir::{BinOpKind, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::declare_lint_pass;
use rustc_span::sym;

declare_clippy_lint! {
/// ### What it does
Expand All @@ -29,31 +28,7 @@ declare_clippy_lint! {
"comparison with an address of a function item"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for comparisons with an address of a trait vtable.
///
/// ### Why is this bad?
/// Comparing trait objects pointers compares an vtable addresses which
/// are not guaranteed to be unique and could vary between different code generation units.
/// Furthermore vtables for different types could have the same address after being merged
/// together.
///
/// ### Example
/// ```rust,ignore
/// let a: Rc<dyn Trait> = ...
/// let b: Rc<dyn Trait> = ...
/// if Rc::ptr_eq(&a, &b) {
/// ...
/// }
/// ```
#[clippy::version = "1.44.0"]
pub VTABLE_ADDRESS_COMPARISONS,
correctness,
"comparison with an address of a trait vtable"
}

declare_lint_pass!(UnnamedAddress => [FN_ADDRESS_COMPARISONS, VTABLE_ADDRESS_COMPARISONS]);
declare_lint_pass!(UnnamedAddress => [FN_ADDRESS_COMPARISONS]);

impl LateLintPass<'_> for UnnamedAddress {
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
Expand All @@ -64,49 +39,10 @@ impl LateLintPass<'_> for UnnamedAddress {
)
}

fn is_trait_ptr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
match cx.typeck_results().expr_ty_adjusted(expr).kind() {
ty::RawPtr(ty::TypeAndMut { ty, .. }) => ty.is_trait(),
_ => false,
}
}

fn is_fn_def(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
matches!(cx.typeck_results().expr_ty(expr).kind(), ty::FnDef(..))
}

if let ExprKind::Binary(binop, left, right) = expr.kind
&& is_comparison(binop.node)
&& is_trait_ptr(cx, left)
&& is_trait_ptr(cx, right)
{
span_lint_and_help(
cx,
VTABLE_ADDRESS_COMPARISONS,
expr.span,
"comparing trait object pointers compares a non-unique vtable address",
None,
"consider extracting and comparing data pointers only",
);
}

if let ExprKind::Call(func, [ref _left, ref _right]) = expr.kind
&& let ExprKind::Path(ref func_qpath) = func.kind
&& let Some(def_id) = cx.qpath_res(func_qpath, func.hir_id).opt_def_id()
&& cx.tcx.is_diagnostic_item(sym::ptr_eq, def_id)
&& let ty_param = cx.typeck_results().node_args(func.hir_id).type_at(0)
&& ty_param.is_trait()
{
span_lint_and_help(
cx,
VTABLE_ADDRESS_COMPARISONS,
expr.span,
"comparing trait object pointers compares a non-unique vtable address",
None,
"consider extracting and comparing data pointers only",
);
}

if let ExprKind::Binary(binop, left, right) = expr.kind
&& is_comparison(binop.node)
&& cx.typeck_results().expr_ty_adjusted(left).is_fn_ptr()
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/rename.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
#![allow(undropped_manually_drops)]
#![allow(unknown_lints)]
#![allow(unused_labels)]
#![allow(ambiguous_wide_pointer_comparisons)]
#![warn(clippy::almost_complete_range)]
#![warn(clippy::disallowed_names)]
#![warn(clippy::blocks_in_if_conditions)]
Expand Down Expand Up @@ -107,5 +108,6 @@
#![warn(undropped_manually_drops)]
#![warn(unknown_lints)]
#![warn(unused_labels)]
#![warn(ambiguous_wide_pointer_comparisons)]

fn main() {}
2 changes: 2 additions & 0 deletions tests/ui/rename.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
#![allow(undropped_manually_drops)]
#![allow(unknown_lints)]
#![allow(unused_labels)]
#![allow(ambiguous_wide_pointer_comparisons)]
#![warn(clippy::almost_complete_letter_range)]
#![warn(clippy::blacklisted_name)]
#![warn(clippy::block_in_if_condition_expr)]
Expand Down Expand Up @@ -107,5 +108,6 @@
#![warn(clippy::undropped_manually_drops)]
#![warn(clippy::unknown_clippy_lints)]
#![warn(clippy::unused_label)]
#![warn(clippy::vtable_address_comparisons)]

fn main() {}
Loading

0 comments on commit c8213a4

Please sign in to comment.