Skip to content

Commit

Permalink
Auto merge of #12713 - J-ZhengLi:issue8864, r=y21
Browse files Browse the repository at this point in the history
make sure the msrv for `const_raw_ptr_deref` is met when linting [`missing_const_for_fn`]

fixes: #8864

---

changelog: make sure the msrv for `const_ptr_deref` is met when linting [`missing_const_for_fn`]
  • Loading branch information
bors committed May 15, 2024
2 parents e669d97 + 1b7fc5f commit caad063
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 36 deletions.
3 changes: 2 additions & 1 deletion clippy_config/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ msrv_aliases! {
1,63,0 { CLONE_INTO }
1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE }
1,59,0 { THREAD_LOCAL_INITIALIZER_CAN_BE_MADE_CONST }
1,58,0 { FORMAT_ARGS_CAPTURE, PATTERN_TRAIT_CHAR_ARRAY }
1,58,0 { FORMAT_ARGS_CAPTURE, PATTERN_TRAIT_CHAR_ARRAY, CONST_RAW_PTR_DEREF }
1,56,0 { CONST_FN_UNION }
1,55,0 { SEEK_REWIND }
1,54,0 { INTO_KEYS }
1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN, ARRAY_INTO_ITERATOR }
Expand Down
78 changes: 45 additions & 33 deletions clippy_utils/src/qualify_min_const_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// of terminologies might not be relevant in the context of Clippy. Note that its behavior might
// differ from the time of `rustc` even if the name stays the same.

use clippy_config::msrvs::Msrv;
use clippy_config::msrvs::{self, Msrv};
use hir::LangItem;
use rustc_attr::StableSince;
use rustc_const_eval::transform::check_consts::ConstCx;
Expand Down Expand Up @@ -42,7 +42,7 @@ pub fn is_min_const_fn<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, msrv: &Msrv)
for bb in &*body.basic_blocks {
check_terminator(tcx, body, bb.terminator(), msrv)?;
for stmt in &bb.statements {
check_statement(tcx, body, def_id, stmt)?;
check_statement(tcx, body, def_id, stmt, msrv)?;
}
}
Ok(())
Expand Down Expand Up @@ -102,13 +102,14 @@ fn check_rvalue<'tcx>(
def_id: DefId,
rvalue: &Rvalue<'tcx>,
span: Span,
msrv: &Msrv,
) -> McfResult {
match rvalue {
Rvalue::ThreadLocalRef(_) => Err((span, "cannot access thread local storage in const fn".into())),
Rvalue::Len(place) | Rvalue::Discriminant(place) | Rvalue::Ref(_, _, place) | Rvalue::AddressOf(_, place) => {
check_place(tcx, *place, span, body)
check_place(tcx, *place, span, body, msrv)
},
Rvalue::CopyForDeref(place) => check_place(tcx, *place, span, body),
Rvalue::CopyForDeref(place) => check_place(tcx, *place, span, body, msrv),
Rvalue::Repeat(operand, _)
| Rvalue::Use(operand)
| Rvalue::Cast(
Expand All @@ -122,7 +123,7 @@ fn check_rvalue<'tcx>(
| CastKind::PointerCoercion(PointerCoercion::MutToConstPointer | PointerCoercion::ArrayToPointer),
operand,
_,
) => check_operand(tcx, operand, span, body),
) => check_operand(tcx, operand, span, body, msrv),
Rvalue::Cast(
CastKind::PointerCoercion(
PointerCoercion::UnsafeFnPointer
Expand All @@ -141,7 +142,7 @@ fn check_rvalue<'tcx>(
};
let unsized_ty = tcx.struct_tail_erasing_lifetimes(pointee_ty, tcx.param_env(def_id));
if let ty::Slice(_) | ty::Str = unsized_ty.kind() {
check_operand(tcx, op, span, body)?;
check_operand(tcx, op, span, body, msrv)?;
// Casting/coercing things to slices is fine.
Ok(())
} else {
Expand All @@ -162,8 +163,8 @@ fn check_rvalue<'tcx>(
)),
// binops are fine on integers
Rvalue::BinaryOp(_, box (lhs, rhs)) | Rvalue::CheckedBinaryOp(_, box (lhs, rhs)) => {
check_operand(tcx, lhs, span, body)?;
check_operand(tcx, rhs, span, body)?;
check_operand(tcx, lhs, span, body, msrv)?;
check_operand(tcx, rhs, span, body, msrv)?;
let ty = lhs.ty(body, tcx);
if ty.is_integral() || ty.is_bool() || ty.is_char() {
Ok(())
Expand All @@ -179,14 +180,14 @@ fn check_rvalue<'tcx>(
Rvalue::UnaryOp(_, operand) => {
let ty = operand.ty(body, tcx);
if ty.is_integral() || ty.is_bool() {
check_operand(tcx, operand, span, body)
check_operand(tcx, operand, span, body, msrv)
} else {
Err((span, "only int and `bool` operations are stable in const fn".into()))
}
},
Rvalue::Aggregate(_, operands) => {
for operand in operands {
check_operand(tcx, operand, span, body)?;
check_operand(tcx, operand, span, body, msrv)?;
}
Ok(())
},
Expand All @@ -198,28 +199,29 @@ fn check_statement<'tcx>(
body: &Body<'tcx>,
def_id: DefId,
statement: &Statement<'tcx>,
msrv: &Msrv,
) -> McfResult {
let span = statement.source_info.span;
match &statement.kind {
StatementKind::Assign(box (place, rval)) => {
check_place(tcx, *place, span, body)?;
check_rvalue(tcx, body, def_id, rval, span)
check_place(tcx, *place, span, body, msrv)?;
check_rvalue(tcx, body, def_id, rval, span, msrv)
},

StatementKind::FakeRead(box (_, place)) => check_place(tcx, *place, span, body),
StatementKind::FakeRead(box (_, place)) => check_place(tcx, *place, span, body, msrv),
// just an assignment
StatementKind::SetDiscriminant { place, .. } | StatementKind::Deinit(place) => {
check_place(tcx, **place, span, body)
check_place(tcx, **place, span, body, msrv)
},

StatementKind::Intrinsic(box NonDivergingIntrinsic::Assume(op)) => check_operand(tcx, op, span, body),
StatementKind::Intrinsic(box NonDivergingIntrinsic::Assume(op)) => check_operand(tcx, op, span, body, msrv),

StatementKind::Intrinsic(box NonDivergingIntrinsic::CopyNonOverlapping(
rustc_middle::mir::CopyNonOverlapping { dst, src, count },
)) => {
check_operand(tcx, dst, span, body)?;
check_operand(tcx, src, span, body)?;
check_operand(tcx, count, span, body)
check_operand(tcx, dst, span, body, msrv)?;
check_operand(tcx, src, span, body, msrv)?;
check_operand(tcx, count, span, body, msrv)
},
// These are all NOPs
StatementKind::StorageLive(_)
Expand All @@ -233,7 +235,13 @@ fn check_statement<'tcx>(
}
}

fn check_operand<'tcx>(tcx: TyCtxt<'tcx>, operand: &Operand<'tcx>, span: Span, body: &Body<'tcx>) -> McfResult {
fn check_operand<'tcx>(
tcx: TyCtxt<'tcx>,
operand: &Operand<'tcx>,
span: Span,
body: &Body<'tcx>,
msrv: &Msrv,
) -> McfResult {
match operand {
Operand::Move(place) => {
if !place.projection.as_ref().is_empty()
Expand All @@ -245,33 +253,37 @@ fn check_operand<'tcx>(tcx: TyCtxt<'tcx>, operand: &Operand<'tcx>, span: Span, b
));
}

check_place(tcx, *place, span, body)
check_place(tcx, *place, span, body, msrv)
},
Operand::Copy(place) => check_place(tcx, *place, span, body),
Operand::Copy(place) => check_place(tcx, *place, span, body, msrv),
Operand::Constant(c) => match c.check_static_ptr(tcx) {
Some(_) => Err((span, "cannot access `static` items in const fn".into())),
None => Ok(()),
},
}
}

fn check_place<'tcx>(tcx: TyCtxt<'tcx>, place: Place<'tcx>, span: Span, body: &Body<'tcx>) -> McfResult {
fn check_place<'tcx>(tcx: TyCtxt<'tcx>, place: Place<'tcx>, span: Span, body: &Body<'tcx>, msrv: &Msrv) -> McfResult {
for (base, elem) in place.as_ref().iter_projections() {
match elem {
ProjectionElem::Field(..) => {
let base_ty = base.ty(body, tcx).ty;
if let Some(def) = base_ty.ty_adt_def() {
// No union field accesses in `const fn`
if def.is_union() {
return Err((span, "accessing union fields is unstable".into()));
}
if base.ty(body, tcx).ty.is_union() && !msrv.meets(msrvs::CONST_FN_UNION) {
return Err((span, "accessing union fields is unstable".into()));
}
},
ProjectionElem::Deref => match base.ty(body, tcx).ty.kind() {
ty::RawPtr(_, hir::Mutability::Mut) => {
return Err((span, "dereferencing raw mut pointer in const fn is unstable".into()));
},
ty::RawPtr(_, hir::Mutability::Not) if !msrv.meets(msrvs::CONST_RAW_PTR_DEREF) => {
return Err((span, "dereferencing raw const pointer in const fn is unstable".into()));
},
_ => (),
},
ProjectionElem::ConstantIndex { .. }
| ProjectionElem::OpaqueCast(..)
| ProjectionElem::Downcast(..)
| ProjectionElem::Subslice { .. }
| ProjectionElem::Deref
| ProjectionElem::Subtype(_)
| ProjectionElem::Index(_) => {},
}
Expand Down Expand Up @@ -304,7 +316,7 @@ fn check_terminator<'tcx>(
}
Ok(())
},
TerminatorKind::SwitchInt { discr, targets: _ } => check_operand(tcx, discr, span, body),
TerminatorKind::SwitchInt { discr, targets: _ } => check_operand(tcx, discr, span, body, msrv),
TerminatorKind::CoroutineDrop | TerminatorKind::Yield { .. } => {
Err((span, "const fn coroutines are unstable".into()))
},
Expand Down Expand Up @@ -341,10 +353,10 @@ fn check_terminator<'tcx>(
));
}

check_operand(tcx, func, span, body)?;
check_operand(tcx, func, span, body, msrv)?;

for arg in args {
check_operand(tcx, &arg.node, span, body)?;
check_operand(tcx, &arg.node, span, body, msrv)?;
}
Ok(())
} else {
Expand All @@ -357,7 +369,7 @@ fn check_terminator<'tcx>(
msg: _,
target: _,
unwind: _,
} => check_operand(tcx, cond, span, body),
} => check_operand(tcx, cond, span, body, msrv),
TerminatorKind::InlineAsm { .. } => Err((span, "cannot use inline assembly in const fn".into())),
}
}
Expand Down
18 changes: 17 additions & 1 deletion tests/ui/missing_const_for_fn/cant_be_const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,23 @@ union U {
f: u32,
}

// Do not lint because accessing union fields from const functions is unstable
// Do not lint because accessing union fields from const functions is unstable in 1.55
#[clippy::msrv = "1.55"]
fn h(u: U) -> u32 {
unsafe { u.f }
}

mod msrv {
struct Foo(*const u8, *mut u8);

impl Foo {
#[clippy::msrv = "1.57"]
fn deref_ptr_cannot_be_const(self) -> usize {
unsafe { *self.0 as usize }
}
#[clippy::msrv = "1.58"]
fn deref_mut_ptr_cannot_be_const(self) -> usize {
unsafe { *self.1 as usize }
}
}
}
28 changes: 28 additions & 0 deletions tests/ui/missing_const_for_fn/could_be_const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,31 @@ impl const Drop for D {
// Lint this, since it can be dropped in const contexts
// FIXME(effects)
fn d(this: D) {}

mod msrv {
struct Foo(*const u8, &'static u8);

impl Foo {
#[clippy::msrv = "1.58"]
fn deref_ptr_can_be_const(self) -> usize {
//~^ ERROR: this could be a `const fn`
unsafe { *self.0 as usize }
}

fn deref_copied_val(self) -> usize {
//~^ ERROR: this could be a `const fn`
*self.1 as usize
}
}

union Bar {
val: u8,
}

#[clippy::msrv = "1.56"]
fn union_access_can_be_const() {
//~^ ERROR: this could be a `const fn`
let bar = Bar { val: 1 };
let _ = unsafe { bar.val };
}
}
30 changes: 29 additions & 1 deletion tests/ui/missing_const_for_fn/could_be_const.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -102,5 +102,33 @@ LL | | 46
LL | | }
| |_^

error: aborting due to 11 previous errors
error: this could be a `const fn`
--> tests/ui/missing_const_for_fn/could_be_const.rs:122:9
|
LL | / fn deref_ptr_can_be_const(self) -> usize {
LL | |
LL | | unsafe { *self.0 as usize }
LL | | }
| |_________^

error: this could be a `const fn`
--> tests/ui/missing_const_for_fn/could_be_const.rs:127:9
|
LL | / fn deref_copied_val(self) -> usize {
LL | |
LL | | *self.1 as usize
LL | | }
| |_________^

error: this could be a `const fn`
--> tests/ui/missing_const_for_fn/could_be_const.rs:138:5
|
LL | / fn union_access_can_be_const() {
LL | |
LL | | let bar = Bar { val: 1 };
LL | | let _ = unsafe { bar.val };
LL | | }
| |_____^

error: aborting due to 14 previous errors

0 comments on commit caad063

Please sign in to comment.