Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make sure the msrv for const_raw_ptr_deref is met when linting [missing_const_for_fn] #12713

Merged
merged 1 commit into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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