Skip to content

[missing_const_for_fn]: Ensure dropped locals are ~const Destruct #10891

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

Merged
merged 5 commits into from
Jun 13, 2023
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.
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
73 changes: 63 additions & 10 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 @@ -256,7 +263,19 @@ 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) => {
if !place.projection.as_ref().is_empty()
&& !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(),
));
}

check_place(tcx, *place, span, body)
},
Operand::Copy(place) => check_place(tcx, *place, span, body),
Operand::Constant(c) => match c.check_static_ptr(tcx) {
Some(_) => Err((span, "cannot access `static` items in const fn".into())),
None => Ok(()),
Expand All @@ -266,6 +285,7 @@ fn check_operand<'tcx>(tcx: TyCtxt<'tcx>, operand: &Operand<'tcx>, span: Span, b

fn check_place<'tcx>(tcx: TyCtxt<'tcx>, place: Place<'tcx>, span: Span, body: &Body<'tcx>) -> McfResult {
let mut cursor = place.projection.as_ref();

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