Skip to content

Commit af4a5a1

Browse files
borsbors
committed
Auto merge of #121268 - Urgau:improve_ambi_wide_ptr_cmps, r=Nadrieril
Add detection of [Partial]Ord methods in the `ambiguous_wide_pointer_comparisons` lint Partially addresses #121264 by adding diagnostics items for PartialOrd and Ord methods, detecting such diagnostics items as "binary operation" and suggesting the correct replacement. I also took the opportunity to change the suggestion to use new methods `.cast()` on `*mut T` an d `*const T`.
2 parents 399fa2f + d4b514f commit af4a5a1

File tree

10 files changed

+271
-82
lines changed

10 files changed

+271
-82
lines changed

Diff for: ‎compiler/rustc_lint/src/lints.rs

+8-6
Original file line numberDiff line numberDiff line change
@@ -1668,14 +1668,16 @@ pub enum AmbiguousWidePointerComparisonsAddrSuggestion<'a> {
16681668
Cast {
16691669
deref_left: &'a str,
16701670
deref_right: &'a str,
1671-
#[suggestion_part(code = "{deref_left}")]
1671+
paren_left: &'a str,
1672+
paren_right: &'a str,
1673+
#[suggestion_part(code = "({deref_left}")]
16721674
left_before: Option<Span>,
1673-
#[suggestion_part(code = " as *const ()")]
1674-
left: Span,
1675-
#[suggestion_part(code = "{deref_right}")]
1675+
#[suggestion_part(code = "{paren_left}.cast::<()>()")]
1676+
left_after: Span,
1677+
#[suggestion_part(code = "({deref_right}")]
16761678
right_before: Option<Span>,
1677-
#[suggestion_part(code = " as *const ()")]
1678-
right: Span,
1679+
#[suggestion_part(code = "{paren_right}.cast::<()>()")]
1680+
right_after: Span,
16791681
},
16801682
}
16811683

Diff for: ‎compiler/rustc_lint/src/types.rs

+35-23
Original file line numberDiff line numberDiff line change
@@ -657,10 +657,16 @@ fn lint_nan<'tcx>(
657657
cx.emit_span_lint(INVALID_NAN_COMPARISONS, e.span, lint);
658658
}
659659

660+
#[derive(Debug, PartialEq)]
661+
enum ComparisonOp {
662+
BinOp(hir::BinOpKind),
663+
Other,
664+
}
665+
660666
fn lint_wide_pointer<'tcx>(
661667
cx: &LateContext<'tcx>,
662668
e: &'tcx hir::Expr<'tcx>,
663-
binop: hir::BinOpKind,
669+
cmpop: ComparisonOp,
664670
l: &'tcx hir::Expr<'tcx>,
665671
r: &'tcx hir::Expr<'tcx>,
666672
) {
@@ -679,7 +685,7 @@ fn lint_wide_pointer<'tcx>(
679685
}
680686
};
681687

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

@@ -707,8 +713,8 @@ fn lint_wide_pointer<'tcx>(
707713
);
708714
};
709715

710-
let ne = if binop == hir::BinOpKind::Ne { "!" } else { "" };
711-
let is_eq_ne = matches!(binop, hir::BinOpKind::Eq | hir::BinOpKind::Ne);
716+
let ne = if cmpop == ComparisonOp::BinOp(hir::BinOpKind::Ne) { "!" } else { "" };
717+
let is_eq_ne = matches!(cmpop, ComparisonOp::BinOp(hir::BinOpKind::Eq | hir::BinOpKind::Ne));
712718
let is_dyn_comparison = l_inner_ty_is_dyn && r_inner_ty_is_dyn;
713719

714720
let left = e.span.shrink_to_lo().until(l_span.shrink_to_lo());
@@ -745,12 +751,12 @@ fn lint_wide_pointer<'tcx>(
745751
AmbiguousWidePointerComparisonsAddrSuggestion::Cast {
746752
deref_left,
747753
deref_right,
748-
// those two Options are required for correctness as having
749-
// an empty span and an empty suggestion is not permitted
750-
left_before: (l_ty_refs != 0).then_some(left),
751-
right_before: (r_ty_refs != 0).then(|| r_span.shrink_to_lo()),
752-
left: l_span.shrink_to_hi(),
753-
right,
754+
paren_left: if l_ty_refs != 0 { ")" } else { "" },
755+
paren_right: if r_ty_refs != 0 { ")" } else { "" },
756+
left_before: (l_ty_refs != 0).then_some(l_span.shrink_to_lo()),
757+
left_after: l_span.shrink_to_hi(),
758+
right_before: (r_ty_refs != 0).then_some(r_span.shrink_to_lo()),
759+
right_after: r_span.shrink_to_hi(),
754760
}
755761
},
756762
},
@@ -773,7 +779,7 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits {
773779
cx.emit_span_lint(UNUSED_COMPARISONS, e.span, UnusedComparisons);
774780
} else {
775781
lint_nan(cx, e, binop, l, r);
776-
lint_wide_pointer(cx, e, binop.node, l, r);
782+
lint_wide_pointer(cx, e, ComparisonOp::BinOp(binop.node), l, r);
777783
}
778784
}
779785
}
@@ -782,16 +788,16 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits {
782788
if let ExprKind::Path(ref qpath) = path.kind
783789
&& let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id()
784790
&& let Some(diag_item) = cx.tcx.get_diagnostic_name(def_id)
785-
&& let Some(binop) = partialeq_binop(diag_item) =>
791+
&& let Some(cmpop) = diag_item_cmpop(diag_item) =>
786792
{
787-
lint_wide_pointer(cx, e, binop, l, r);
793+
lint_wide_pointer(cx, e, cmpop, l, r);
788794
}
789795
hir::ExprKind::MethodCall(_, l, [r], _)
790796
if let Some(def_id) = cx.typeck_results().type_dependent_def_id(e.hir_id)
791797
&& let Some(diag_item) = cx.tcx.get_diagnostic_name(def_id)
792-
&& let Some(binop) = partialeq_binop(diag_item) =>
798+
&& let Some(cmpop) = diag_item_cmpop(diag_item) =>
793799
{
794-
lint_wide_pointer(cx, e, binop, l, r);
800+
lint_wide_pointer(cx, e, cmpop, l, r);
795801
}
796802
_ => {}
797803
};
@@ -876,14 +882,20 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits {
876882
)
877883
}
878884

879-
fn partialeq_binop(diag_item: Symbol) -> Option<hir::BinOpKind> {
880-
if diag_item == sym::cmp_partialeq_eq {
881-
Some(hir::BinOpKind::Eq)
882-
} else if diag_item == sym::cmp_partialeq_ne {
883-
Some(hir::BinOpKind::Ne)
884-
} else {
885-
None
886-
}
885+
fn diag_item_cmpop(diag_item: Symbol) -> Option<ComparisonOp> {
886+
Some(match diag_item {
887+
sym::cmp_ord_max => ComparisonOp::Other,
888+
sym::cmp_ord_min => ComparisonOp::Other,
889+
sym::ord_cmp_method => ComparisonOp::Other,
890+
sym::cmp_partialeq_eq => ComparisonOp::BinOp(hir::BinOpKind::Eq),
891+
sym::cmp_partialeq_ne => ComparisonOp::BinOp(hir::BinOpKind::Ne),
892+
sym::cmp_partialord_cmp => ComparisonOp::Other,
893+
sym::cmp_partialord_ge => ComparisonOp::BinOp(hir::BinOpKind::Ge),
894+
sym::cmp_partialord_gt => ComparisonOp::BinOp(hir::BinOpKind::Gt),
895+
sym::cmp_partialord_le => ComparisonOp::BinOp(hir::BinOpKind::Le),
896+
sym::cmp_partialord_lt => ComparisonOp::BinOp(hir::BinOpKind::Lt),
897+
_ => return None,
898+
})
887899
}
888900
}
889901
}

Diff for: ‎compiler/rustc_span/src/symbol.rs

+7
Original file line numberDiff line numberDiff line change
@@ -531,8 +531,15 @@ symbols! {
531531
cmp,
532532
cmp_max,
533533
cmp_min,
534+
cmp_ord_max,
535+
cmp_ord_min,
534536
cmp_partialeq_eq,
535537
cmp_partialeq_ne,
538+
cmp_partialord_cmp,
539+
cmp_partialord_ge,
540+
cmp_partialord_gt,
541+
cmp_partialord_le,
542+
cmp_partialord_lt,
536543
cmpxchg16b_target_feature,
537544
cmse_nonsecure_entry,
538545
coerce_unsized,

Diff for: ‎library/core/src/cmp.rs

+7
Original file line numberDiff line numberDiff line change
@@ -848,6 +848,7 @@ pub trait Ord: Eq + PartialOrd<Self> {
848848
#[stable(feature = "ord_max_min", since = "1.21.0")]
849849
#[inline]
850850
#[must_use]
851+
#[cfg_attr(not(bootstrap), rustc_diagnostic_item = "cmp_ord_max")]
851852
fn max(self, other: Self) -> Self
852853
where
853854
Self: Sized,
@@ -868,6 +869,7 @@ pub trait Ord: Eq + PartialOrd<Self> {
868869
#[stable(feature = "ord_max_min", since = "1.21.0")]
869870
#[inline]
870871
#[must_use]
872+
#[cfg_attr(not(bootstrap), rustc_diagnostic_item = "cmp_ord_min")]
871873
fn min(self, other: Self) -> Self
872874
where
873875
Self: Sized,
@@ -1154,6 +1156,7 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
11541156
/// ```
11551157
#[must_use]
11561158
#[stable(feature = "rust1", since = "1.0.0")]
1159+
#[cfg_attr(not(bootstrap), rustc_diagnostic_item = "cmp_partialord_cmp")]
11571160
fn partial_cmp(&self, other: &Rhs) -> Option<Ordering>;
11581161

11591162
/// This method tests less than (for `self` and `other`) and is used by the `<` operator.
@@ -1168,6 +1171,7 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
11681171
#[inline]
11691172
#[must_use]
11701173
#[stable(feature = "rust1", since = "1.0.0")]
1174+
#[cfg_attr(not(bootstrap), rustc_diagnostic_item = "cmp_partialord_lt")]
11711175
fn lt(&self, other: &Rhs) -> bool {
11721176
matches!(self.partial_cmp(other), Some(Less))
11731177
}
@@ -1185,6 +1189,7 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
11851189
#[inline]
11861190
#[must_use]
11871191
#[stable(feature = "rust1", since = "1.0.0")]
1192+
#[cfg_attr(not(bootstrap), rustc_diagnostic_item = "cmp_partialord_le")]
11881193
fn le(&self, other: &Rhs) -> bool {
11891194
matches!(self.partial_cmp(other), Some(Less | Equal))
11901195
}
@@ -1201,6 +1206,7 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
12011206
#[inline]
12021207
#[must_use]
12031208
#[stable(feature = "rust1", since = "1.0.0")]
1209+
#[cfg_attr(not(bootstrap), rustc_diagnostic_item = "cmp_partialord_gt")]
12041210
fn gt(&self, other: &Rhs) -> bool {
12051211
matches!(self.partial_cmp(other), Some(Greater))
12061212
}
@@ -1218,6 +1224,7 @@ pub trait PartialOrd<Rhs: ?Sized = Self>: PartialEq<Rhs> {
12181224
#[inline]
12191225
#[must_use]
12201226
#[stable(feature = "rust1", since = "1.0.0")]
1227+
#[cfg_attr(not(bootstrap), rustc_diagnostic_item = "cmp_partialord_ge")]
12211228
fn ge(&self, other: &Rhs) -> bool {
12221229
matches!(self.partial_cmp(other), Some(Greater | Equal))
12231230
}

Diff for: ‎library/core/src/ptr/const_ptr.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1857,6 +1857,7 @@ impl<T: ?Sized> Ord for *const T {
18571857
#[stable(feature = "rust1", since = "1.0.0")]
18581858
impl<T: ?Sized> PartialOrd for *const T {
18591859
#[inline]
1860+
#[allow(ambiguous_wide_pointer_comparisons)]
18601861
fn partial_cmp(&self, other: &*const T) -> Option<Ordering> {
18611862
Some(self.cmp(other))
18621863
}

Diff for: ‎library/core/src/ptr/metadata.rs

+1
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ impl<Dyn: ?Sized> PartialEq for DynMetadata<Dyn> {
258258

259259
impl<Dyn: ?Sized> Ord for DynMetadata<Dyn> {
260260
#[inline]
261+
#[allow(ambiguous_wide_pointer_comparisons)]
261262
fn cmp(&self, other: &Self) -> crate::cmp::Ordering {
262263
(self.vtable_ptr as *const VTable).cmp(&(other.vtable_ptr as *const VTable))
263264
}

Diff for: ‎library/core/src/ptr/mut_ptr.rs

+1
Original file line numberDiff line numberDiff line change
@@ -2275,6 +2275,7 @@ impl<T: ?Sized> Ord for *mut T {
22752275
#[stable(feature = "rust1", since = "1.0.0")]
22762276
impl<T: ?Sized> PartialOrd for *mut T {
22772277
#[inline(always)]
2278+
#[allow(ambiguous_wide_pointer_comparisons)]
22782279
fn partial_cmp(&self, other: &*mut T) -> Option<Ordering> {
22792280
Some(self.cmp(other))
22802281
}

Diff for: ‎library/core/src/ptr/non_null.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1821,6 +1821,7 @@ impl<T: ?Sized> PartialEq for NonNull<T> {
18211821
#[stable(feature = "nonnull", since = "1.25.0")]
18221822
impl<T: ?Sized> Ord for NonNull<T> {
18231823
#[inline]
1824+
#[allow(ambiguous_wide_pointer_comparisons)]
18241825
fn cmp(&self, other: &Self) -> Ordering {
18251826
self.as_ptr().cmp(&other.as_ptr())
18261827
}
@@ -1829,6 +1830,7 @@ impl<T: ?Sized> Ord for NonNull<T> {
18291830
#[stable(feature = "nonnull", since = "1.25.0")]
18301831
impl<T: ?Sized> PartialOrd for NonNull<T> {
18311832
#[inline]
1833+
#[allow(ambiguous_wide_pointer_comparisons)]
18321834
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
18331835
self.as_ptr().partial_cmp(&other.as_ptr())
18341836
}

Diff for: ‎tests/ui/lint/wide_pointer_comparisons.rs

+24
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,18 @@ fn main() {
3737
//~^ WARN ambiguous wide pointer comparison
3838
let _ = a.ne(&b);
3939
//~^ WARN ambiguous wide pointer comparison
40+
let _ = a.cmp(&b);
41+
//~^ WARN ambiguous wide pointer comparison
42+
let _ = a.partial_cmp(&b);
43+
//~^ WARN ambiguous wide pointer comparison
44+
let _ = a.le(&b);
45+
//~^ WARN ambiguous wide pointer comparison
46+
let _ = a.lt(&b);
47+
//~^ WARN ambiguous wide pointer comparison
48+
let _ = a.ge(&b);
49+
//~^ WARN ambiguous wide pointer comparison
50+
let _ = a.gt(&b);
51+
//~^ WARN ambiguous wide pointer comparison
4052

4153
{
4254
// &*const ?Sized
@@ -68,6 +80,18 @@ fn main() {
6880
//~^ WARN ambiguous wide pointer comparison
6981
let _ = a.ne(b);
7082
//~^ WARN ambiguous wide pointer comparison
83+
let _ = a.cmp(&b);
84+
//~^ WARN ambiguous wide pointer comparison
85+
let _ = a.partial_cmp(&b);
86+
//~^ WARN ambiguous wide pointer comparison
87+
let _ = a.le(&b);
88+
//~^ WARN ambiguous wide pointer comparison
89+
let _ = a.lt(&b);
90+
//~^ WARN ambiguous wide pointer comparison
91+
let _ = a.ge(&b);
92+
//~^ WARN ambiguous wide pointer comparison
93+
let _ = a.gt(&b);
94+
//~^ WARN ambiguous wide pointer comparison
7195
}
7296

7397
let s = "" as *const str;

0 commit comments

Comments
 (0)