Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
extern crate rustc_ast;
extern crate rustc_ast_pretty;
extern crate rustc_attr;
extern crate rustc_const_eval;
extern crate rustc_data_structures;
// The `rustc_driver` crate seems to be required in order to use the `rust_ast` crate.
#[allow(unused_extern_crates)]
Expand Down
85 changes: 69 additions & 16 deletions clippy_utils/src/qualify_min_const_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,24 @@
// differ from the time of `rustc` even if the name stays the same.

use crate::msrvs::Msrv;
use hir::LangItem;
use rustc_const_eval::transform::check_consts::ConstCx;
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_infer::traits::Obligation;
use rustc_middle::mir::{
Body, CastKind, NonDivergingIntrinsic, NullOp, Operand, Place, ProjectionElem, Rvalue, Statement, StatementKind,
Terminator, TerminatorKind,
};
use rustc_middle::traits::{ImplSource, ObligationCause};
use rustc_middle::ty::subst::GenericArgKind;
use rustc_middle::ty::{self, adjustment::PointerCast, Ty, TyCtxt};
use rustc_middle::ty::{BoundConstness, TraitRef};
use rustc_semver::RustcVersion;
use rustc_span::symbol::sym;
use rustc_span::Span;
use rustc_trait_selection::traits::{ObligationCtxt, SelectionContext};
use std::borrow::Cow;

type McfResult = Result<(), (Span, Cow<'static, str>)>;
Expand Down Expand Up @@ -128,9 +135,9 @@ fn check_rvalue<'tcx>(
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, false)
},
Rvalue::CopyForDeref(place) => check_place(tcx, *place, span, body),
Rvalue::CopyForDeref(place) => check_place(tcx, *place, span, body, false),
Rvalue::Repeat(operand, _)
| Rvalue::Use(operand)
| Rvalue::Cast(
Expand Down Expand Up @@ -223,14 +230,14 @@ fn check_statement<'tcx>(
let span = statement.source_info.span;
match &statement.kind {
StatementKind::Assign(box (place, rval)) => {
check_place(tcx, *place, span, body)?;
check_place(tcx, *place, span, body, false)?;
check_rvalue(tcx, body, def_id, rval, span)
},

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

StatementKind::Intrinsic(box NonDivergingIntrinsic::Assume(op)) => check_operand(tcx, op, span, body),
Expand All @@ -256,16 +263,29 @@ fn check_statement<'tcx>(

fn check_operand<'tcx>(tcx: TyCtxt<'tcx>, operand: &Operand<'tcx>, span: Span, body: &Body<'tcx>) -> McfResult {
match operand {
Operand::Move(place) | Operand::Copy(place) => check_place(tcx, *place, span, body),
Operand::Move(place) => check_place(tcx, *place, span, body, true),
Operand::Copy(place) => check_place(tcx, *place, span, body, false),
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>, in_move: bool) -> McfResult {
Comment thread
Jarcho marked this conversation as resolved.
Outdated
let mut cursor = place.projection.as_ref();

if let [proj_base] = cursor
&& let ProjectionElem::Field(_, ty) = proj_base
&& !is_ty_const_destruct(tcx, *ty, body)
&& in_move
Comment thread
Centri3 marked this conversation as resolved.
Outdated
{
return Err((
span,
"cannot drop locals with a non constant destructor in const fn".into(),
));
}

while let [ref proj_base @ .., elem] = *cursor {
cursor = proj_base;
match elem {
Expand Down Expand Up @@ -305,15 +325,19 @@ fn check_terminator<'tcx>(
| TerminatorKind::Resume
| TerminatorKind::Terminate
| TerminatorKind::Unreachable => Ok(()),

TerminatorKind::Drop { place, .. } => check_place(tcx, *place, span, body),

TerminatorKind::Drop { place, .. } => {
if !is_ty_const_destruct(tcx, place.ty(&body.local_decls, tcx).ty, body) {
return Err((
span,
"cannot drop locals with a non constant destructor in const fn".into(),
));
}
Ok(())
},
TerminatorKind::SwitchInt { discr, targets: _ } => check_operand(tcx, discr, span, body),

TerminatorKind::GeneratorDrop | TerminatorKind::Yield { .. } => {
Err((span, "const fn generators are unstable".into()))
},

TerminatorKind::Call {
func,
args,
Expand Down Expand Up @@ -357,15 +381,13 @@ fn check_terminator<'tcx>(
Err((span, "can only call other const fns within const fn".into()))
}
},

TerminatorKind::Assert {
cond,
expected: _,
msg: _,
target: _,
unwind: _,
} => check_operand(tcx, cond, span, body),

TerminatorKind::InlineAsm { .. } => Err((span, "cannot use inline assembly in const fn".into())),
}
}
Expand All @@ -379,8 +401,7 @@ fn is_const_fn(tcx: TyCtxt<'_>, def_id: DefId, msrv: &Msrv) -> bool {
// as a part of an unimplemented MSRV check https://github.com/rust-lang/rust/issues/65262.

// HACK(nilstrieb): CURRENT_RUSTC_VERSION can return versions like 1.66.0-dev. `rustc-semver`
// doesn't accept the `-dev` version number so we have to strip it
// off.
// doesn't accept the `-dev` version number so we have to strip it off.
let short_version = since
.as_str()
.split('-')
Expand All @@ -398,3 +419,35 @@ fn is_const_fn(tcx: TyCtxt<'_>, def_id: DefId, msrv: &Msrv) -> bool {
}
})
}

#[expect(clippy::similar_names)] // bit too pedantic
fn is_ty_const_destruct<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, body: &Body<'tcx>) -> bool {
// Avoid selecting for simple cases, such as builtin types.
if ty::util::is_trivially_const_drop(ty) {
return true;
}

let obligation = Obligation::new(
tcx,
ObligationCause::dummy_with_span(body.span),
ConstCx::new(tcx, body).param_env.with_const(),
TraitRef::from_lang_item(tcx, LangItem::Destruct, body.span, [ty]).with_constness(BoundConstness::ConstIfConst),
);

let infcx = tcx.infer_ctxt().build();
let mut selcx = SelectionContext::new(&infcx);
let Some(impl_src) = selcx.select(&obligation).ok().flatten() else {
return false;
};

if !matches!(
impl_src,
ImplSource::ConstDestruct(_) | ImplSource::Param(_, ty::BoundConstness::ConstIfConst)
) {
return false;
}

let ocx = ObligationCtxt::new(&infcx);
ocx.register_obligations(impl_src.nested_obligations());
ocx.select_all_or_error().is_empty()
}
33 changes: 32 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 @@ -13,7 +13,7 @@ extern crate proc_macros;

use proc_macros::with_span;

struct Game;
struct Game; // You just lost.

// This should not be linted because it's already const
const fn already_const() -> i32 {
Expand Down Expand Up @@ -126,3 +126,34 @@ with_span! {
span
fn dont_check_in_proc_macro() {}
}

// Do not lint `String` has `Vec<u8>`, which cannot be dropped in const contexts
fn a(this: String) {}

enum A {
F(String),
N,
}

// Same here.
fn b(this: A) {}

// Minimized version of `a`.
fn c(this: Vec<u16>) {}

struct F(A);

// Do not lint
fn f(this: F) {}

// Do not lint
fn g<T>(this: T) {}

struct Issue10617(String);

impl Issue10617 {
// Do not lint
pub fn name(self) -> String {
self.0
}
}
13 changes: 13 additions & 0 deletions tests/ui/missing_const_for_fn/could_be_const.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#![warn(clippy::missing_const_for_fn)]
#![allow(incomplete_features, clippy::let_and_return)]
#![feature(const_mut_refs)]
#![feature(const_trait_impl)]

use std::mem::transmute;

Expand Down Expand Up @@ -87,3 +89,14 @@ fn msrv_1_46() -> i32 {

// Should not be const
fn main() {}

struct D;

impl const Drop for D {
fn drop(&mut self) {
todo!();
}
}

// Lint this, since it can be dropped in const contexts
fn d(this: D) {}
30 changes: 18 additions & 12 deletions tests/ui/missing_const_for_fn/could_be_const.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: this could be a `const fn`
--> $DIR/could_be_const.rs:12:5
--> $DIR/could_be_const.rs:14:5
|
LL | / pub fn new() -> Self {
LL | | Self { guess: 42 }
Expand All @@ -9,23 +9,23 @@ LL | | }
= note: `-D clippy::missing-const-for-fn` implied by `-D warnings`

error: this could be a `const fn`
--> $DIR/could_be_const.rs:16:5
--> $DIR/could_be_const.rs:18:5
|
LL | / fn const_generic_params<'a, T, const N: usize>(&self, b: &'a [T; N]) -> &'a [T; N] {
LL | | b
LL | | }
| |_____^

error: this could be a `const fn`
--> $DIR/could_be_const.rs:22:1
--> $DIR/could_be_const.rs:24:1
|
LL | / fn one() -> i32 {
LL | | 1
LL | | }
| |_^

error: this could be a `const fn`
--> $DIR/could_be_const.rs:27:1
--> $DIR/could_be_const.rs:29:1
|
LL | / fn two() -> i32 {
LL | | let abc = 2;
Expand All @@ -34,60 +34,66 @@ LL | | }
| |_^

error: this could be a `const fn`
--> $DIR/could_be_const.rs:33:1
--> $DIR/could_be_const.rs:35:1
|
LL | / fn string() -> String {
LL | | String::new()
LL | | }
| |_^

error: this could be a `const fn`
--> $DIR/could_be_const.rs:38:1
--> $DIR/could_be_const.rs:40:1
|
LL | / unsafe fn four() -> i32 {
LL | | 4
LL | | }
| |_^

error: this could be a `const fn`
--> $DIR/could_be_const.rs:43:1
--> $DIR/could_be_const.rs:45:1
|
LL | / fn generic<T>(t: T) -> T {
LL | | t
LL | | }
| |_^

error: this could be a `const fn`
--> $DIR/could_be_const.rs:51:1
--> $DIR/could_be_const.rs:53:1
|
LL | / fn generic_arr<T: Copy>(t: [T; 1]) -> T {
LL | | t[0]
LL | | }
| |_^

error: this could be a `const fn`
--> $DIR/could_be_const.rs:64:9
--> $DIR/could_be_const.rs:66:9
|
LL | / pub fn b(self, a: &A) -> B {
LL | | B
LL | | }
| |_________^

error: this could be a `const fn`
--> $DIR/could_be_const.rs:73:5
--> $DIR/could_be_const.rs:75:5
|
LL | / fn const_fn_stabilized_before_msrv(byte: u8) {
LL | | byte.is_ascii_digit();
LL | | }
| |_____^

error: this could be a `const fn`
--> $DIR/could_be_const.rs:84:1
--> $DIR/could_be_const.rs:86:1
|
LL | / fn msrv_1_46() -> i32 {
LL | | 46
LL | | }
| |_^

error: aborting due to 11 previous errors
error: this could be a `const fn`
--> $DIR/could_be_const.rs:102:1
|
LL | fn d(this: D) {}
| ^^^^^^^^^^^^^^^^

error: aborting due to 12 previous errors