Skip to content

Commit

Permalink
Add detection of [Partial]Ord methods to the ambiguous wide ptr cmp lint
Browse files Browse the repository at this point in the history
  • Loading branch information
Urgau committed Feb 18, 2024
1 parent efe3735 commit 5a613c9
Show file tree
Hide file tree
Showing 8 changed files with 248 additions and 79 deletions.
14 changes: 8 additions & 6 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1583,14 +1583,16 @@ pub enum AmbiguousWidePointerComparisonsAddrSuggestion<'a> {
Cast {
deref_left: &'a str,
deref_right: &'a str,
#[suggestion_part(code = "{deref_left}")]
paren_left: &'a str,
paren_right: &'a str,
#[suggestion_part(code = "({deref_left}")]
left_before: Option<Span>,
#[suggestion_part(code = " as *const ()")]
left: Span,
#[suggestion_part(code = "{deref_right}")]
#[suggestion_part(code = "{paren_left}.cast::<()>()")]
left_after: Span,
#[suggestion_part(code = "({deref_right}")]
right_before: Option<Span>,
#[suggestion_part(code = " as *const ()")]
right: Span,
#[suggestion_part(code = "{paren_right}.cast::<()>()")]
right_after: Span,
},
}

Expand Down
46 changes: 26 additions & 20 deletions compiler/rustc_lint/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,7 @@ fn lint_nan<'tcx>(
fn lint_wide_pointer<'tcx>(
cx: &LateContext<'tcx>,
e: &'tcx hir::Expr<'tcx>,
binop: hir::BinOpKind,
binop: Option<hir::BinOpKind>,
l: &'tcx hir::Expr<'tcx>,
r: &'tcx hir::Expr<'tcx>,
) {
Expand All @@ -676,7 +676,7 @@ fn lint_wide_pointer<'tcx>(
}
};

// PartialEq::{eq,ne} takes references, remove any explicit references
// the left and right operands can have references, remove any explicit references
let l = l.peel_borrows();
let r = r.peel_borrows();

Expand Down Expand Up @@ -704,8 +704,8 @@ fn lint_wide_pointer<'tcx>(
);
};

let ne = if binop == hir::BinOpKind::Ne { "!" } else { "" };
let is_eq_ne = matches!(binop, hir::BinOpKind::Eq | hir::BinOpKind::Ne);
let ne = if binop == Some(hir::BinOpKind::Ne) { "!" } else { "" };
let is_eq_ne = matches!(binop, Some(hir::BinOpKind::Eq | hir::BinOpKind::Ne));
let is_dyn_comparison = l_inner_ty_is_dyn && r_inner_ty_is_dyn;

let left = e.span.shrink_to_lo().until(l_span.shrink_to_lo());
Expand Down Expand Up @@ -742,12 +742,12 @@ fn lint_wide_pointer<'tcx>(
AmbiguousWidePointerComparisonsAddrSuggestion::Cast {
deref_left,
deref_right,
// those two Options are required for correctness as having
// an empty span and an empty suggestion is not permitted
left_before: (l_ty_refs != 0).then_some(left),
right_before: (r_ty_refs != 0).then(|| r_span.shrink_to_lo()),
left: l_span.shrink_to_hi(),
right,
paren_left: (l_ty_refs != 0).then_some(")").unwrap_or_default(),
paren_right: (r_ty_refs != 0).then_some(")").unwrap_or_default(),
left_before: (l_ty_refs != 0).then_some(l_span.shrink_to_lo()),
left_after: l_span.shrink_to_hi(),
right_before: (r_ty_refs != 0).then_some(r_span.shrink_to_lo()),
right_after: r_span.shrink_to_hi(),
}
},
},
Expand All @@ -770,7 +770,7 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits {
cx.emit_span_lint(UNUSED_COMPARISONS, e.span, UnusedComparisons);
} else {
lint_nan(cx, e, binop, l, r);
lint_wide_pointer(cx, e, binop.node, l, r);
lint_wide_pointer(cx, e, Some(binop.node), l, r);
}
}
}
Expand All @@ -779,14 +779,14 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits {
if let ExprKind::Path(ref qpath) = path.kind
&& let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id()
&& let Some(diag_item) = cx.tcx.get_diagnostic_name(def_id)
&& let Some(binop) = partialeq_binop(diag_item) =>
&& let Some(binop) = diag_item_binop(diag_item) =>
{
lint_wide_pointer(cx, e, binop, l, r);
}
hir::ExprKind::MethodCall(_, l, [r], _)
if let Some(def_id) = cx.typeck_results().type_dependent_def_id(e.hir_id)
&& let Some(diag_item) = cx.tcx.get_diagnostic_name(def_id)
&& let Some(binop) = partialeq_binop(diag_item) =>
&& let Some(binop) = diag_item_binop(diag_item) =>
{
lint_wide_pointer(cx, e, binop, l, r);
}
Expand Down Expand Up @@ -873,13 +873,19 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits {
)
}

fn partialeq_binop(diag_item: Symbol) -> Option<hir::BinOpKind> {
if diag_item == sym::cmp_partialeq_eq {
Some(hir::BinOpKind::Eq)
} else if diag_item == sym::cmp_partialeq_ne {
Some(hir::BinOpKind::Ne)
} else {
None
fn diag_item_binop(diag_item: Symbol) -> Option<Option<hir::BinOpKind>> {
match diag_item {
sym::cmp_ord_max => Some(None),
sym::cmp_ord_min => Some(None),
sym::ord_cmp_method => Some(None),
sym::cmp_partialeq_eq => Some(Some(hir::BinOpKind::Eq)),
sym::cmp_partialeq_ne => Some(Some(hir::BinOpKind::Ne)),
sym::cmp_partialord_cmp => Some(None),
sym::cmp_partialord_ge => Some(Some(hir::BinOpKind::Ge)),
sym::cmp_partialord_gt => Some(Some(hir::BinOpKind::Gt)),
sym::cmp_partialord_le => Some(Some(hir::BinOpKind::Le)),
sym::cmp_partialord_lt => Some(Some(hir::BinOpKind::Lt)),
_ => None,
}
}
}
Expand Down
1 change: 1 addition & 0 deletions library/core/src/ptr/const_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1802,6 +1802,7 @@ impl<T: ?Sized> Ord for *const T {
#[stable(feature = "rust1", since = "1.0.0")]
impl<T: ?Sized> PartialOrd for *const T {
#[inline]
#[allow(ambiguous_wide_pointer_comparisons)]
fn partial_cmp(&self, other: &*const T) -> Option<Ordering> {
Some(self.cmp(other))
}
Expand Down
1 change: 1 addition & 0 deletions library/core/src/ptr/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ impl<Dyn: ?Sized> PartialEq for DynMetadata<Dyn> {

impl<Dyn: ?Sized> Ord for DynMetadata<Dyn> {
#[inline]
#[allow(ambiguous_wide_pointer_comparisons)]
fn cmp(&self, other: &Self) -> crate::cmp::Ordering {
(self.vtable_ptr as *const VTable).cmp(&(other.vtable_ptr as *const VTable))
}
Expand Down
1 change: 1 addition & 0 deletions library/core/src/ptr/mut_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2226,6 +2226,7 @@ impl<T: ?Sized> Ord for *mut T {
#[stable(feature = "rust1", since = "1.0.0")]
impl<T: ?Sized> PartialOrd for *mut T {
#[inline(always)]
#[allow(ambiguous_wide_pointer_comparisons)]
fn partial_cmp(&self, other: &*mut T) -> Option<Ordering> {
Some(self.cmp(other))
}
Expand Down
2 changes: 2 additions & 0 deletions library/core/src/ptr/non_null.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1802,6 +1802,7 @@ impl<T: ?Sized> PartialEq for NonNull<T> {
#[stable(feature = "nonnull", since = "1.25.0")]
impl<T: ?Sized> Ord for NonNull<T> {
#[inline]
#[allow(ambiguous_wide_pointer_comparisons)]
fn cmp(&self, other: &Self) -> Ordering {
self.as_ptr().cmp(&other.as_ptr())
}
Expand All @@ -1810,6 +1811,7 @@ impl<T: ?Sized> Ord for NonNull<T> {
#[stable(feature = "nonnull", since = "1.25.0")]
impl<T: ?Sized> PartialOrd for NonNull<T> {
#[inline]
#[allow(ambiguous_wide_pointer_comparisons)]
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
self.as_ptr().partial_cmp(&other.as_ptr())
}
Expand Down
24 changes: 24 additions & 0 deletions tests/ui/lint/wide_pointer_comparisons.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,18 @@ fn main() {
//~^ WARN ambiguous wide pointer comparison
let _ = a.ne(&b);
//~^ WARN ambiguous wide pointer comparison
let _ = a.cmp(&b);
//~^ WARN ambiguous wide pointer comparison
let _ = a.partial_cmp(&b);
//~^ WARN ambiguous wide pointer comparison
let _ = a.le(&b);
//~^ WARN ambiguous wide pointer comparison
let _ = a.lt(&b);
//~^ WARN ambiguous wide pointer comparison
let _ = a.ge(&b);
//~^ WARN ambiguous wide pointer comparison
let _ = a.gt(&b);
//~^ WARN ambiguous wide pointer comparison

{
// &*const ?Sized
Expand Down Expand Up @@ -68,6 +80,18 @@ fn main() {
//~^ WARN ambiguous wide pointer comparison
let _ = a.ne(b);
//~^ WARN ambiguous wide pointer comparison
let _ = a.cmp(&b);
//~^ WARN ambiguous wide pointer comparison
let _ = a.partial_cmp(&b);
//~^ WARN ambiguous wide pointer comparison
let _ = a.le(&b);
//~^ WARN ambiguous wide pointer comparison
let _ = a.lt(&b);
//~^ WARN ambiguous wide pointer comparison
let _ = a.ge(&b);
//~^ WARN ambiguous wide pointer comparison
let _ = a.gt(&b);
//~^ WARN ambiguous wide pointer comparison
}

let s = "" as *const str;
Expand Down
Loading

0 comments on commit 5a613c9

Please sign in to comment.