Skip to content

Commit 5692677

Browse files
committed
cleanup spaghetti code
1 parent 0e23349 commit 5692677

File tree

5 files changed

+88
-39
lines changed

5 files changed

+88
-39
lines changed

clippy_utils/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@
2020
extern crate rustc_ast;
2121
extern crate rustc_ast_pretty;
2222
extern crate rustc_attr;
23-
extern crate rustc_data_structures;
2423
extern crate rustc_const_eval;
24+
extern crate rustc_data_structures;
2525
// The `rustc_driver` crate seems to be required in order to use the `rust_ast` crate.
2626
#[allow(unused_extern_crates)]
2727
extern crate rustc_driver;

clippy_utils/src/qualify_min_const_fn.rs

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
// differ from the time of `rustc` even if the name stays the same.
55

66
use crate::msrvs::Msrv;
7-
use hir::{Constness, LangItem};
7+
use hir::LangItem;
88
use rustc_const_eval::transform::check_consts::ConstCx;
99
use rustc_hir as hir;
1010
use rustc_hir::def_id::DefId;
@@ -14,14 +14,14 @@ use rustc_middle::mir::{
1414
Body, CastKind, NonDivergingIntrinsic, NullOp, Operand, Place, ProjectionElem, Rvalue, Statement, StatementKind,
1515
Terminator, TerminatorKind,
1616
};
17-
use rustc_middle::traits::ObligationCause;
17+
use rustc_middle::traits::{ImplSource, ObligationCause};
1818
use rustc_middle::ty::subst::GenericArgKind;
1919
use rustc_middle::ty::{self, adjustment::PointerCast, Ty, TyCtxt};
2020
use rustc_middle::ty::{BoundConstness, TraitRef};
2121
use rustc_semver::RustcVersion;
2222
use rustc_span::symbol::sym;
2323
use rustc_span::Span;
24-
use rustc_trait_selection::traits::SelectionContext;
24+
use rustc_trait_selection::traits::{ObligationCtxt, SelectionContext};
2525
use std::borrow::Cow;
2626

2727
type McfResult = Result<(), (Span, Cow<'static, str>)>;
@@ -135,9 +135,9 @@ fn check_rvalue<'tcx>(
135135
match rvalue {
136136
Rvalue::ThreadLocalRef(_) => Err((span, "cannot access thread local storage in const fn".into())),
137137
Rvalue::Len(place) | Rvalue::Discriminant(place) | Rvalue::Ref(_, _, place) | Rvalue::AddressOf(_, place) => {
138-
check_place(tcx, *place, span, body)
138+
check_place(tcx, *place, span, body, false)
139139
},
140-
Rvalue::CopyForDeref(place) => check_place(tcx, *place, span, body),
140+
Rvalue::CopyForDeref(place) => check_place(tcx, *place, span, body, false),
141141
Rvalue::Repeat(operand, _)
142142
| Rvalue::Use(operand)
143143
| Rvalue::Cast(
@@ -230,14 +230,14 @@ fn check_statement<'tcx>(
230230
let span = statement.source_info.span;
231231
match &statement.kind {
232232
StatementKind::Assign(box (place, rval)) => {
233-
check_place(tcx, *place, span, body)?;
233+
check_place(tcx, *place, span, body, false)?;
234234
check_rvalue(tcx, body, def_id, rval, span)
235235
},
236236

237-
StatementKind::FakeRead(box (_, place)) => check_place(tcx, *place, span, body),
237+
StatementKind::FakeRead(box (_, place)) => check_place(tcx, *place, span, body, false),
238238
// just an assignment
239239
StatementKind::SetDiscriminant { place, .. } | StatementKind::Deinit(place) => {
240-
check_place(tcx, **place, span, body)
240+
check_place(tcx, **place, span, body, false)
241241
},
242242

243243
StatementKind::Intrinsic(box NonDivergingIntrinsic::Assume(op)) => check_operand(tcx, op, span, body),
@@ -263,21 +263,29 @@ fn check_statement<'tcx>(
263263

264264
fn check_operand<'tcx>(tcx: TyCtxt<'tcx>, operand: &Operand<'tcx>, span: Span, body: &Body<'tcx>) -> McfResult {
265265
match operand {
266-
Operand::Move(place) | Operand::Copy(place) => check_place(tcx, *place, span, body),
266+
Operand::Move(place) => check_place(tcx, *place, span, body, true),
267+
Operand::Copy(place) => check_place(tcx, *place, span, body, false),
267268
Operand::Constant(c) => match c.check_static_ptr(tcx) {
268269
Some(_) => Err((span, "cannot access `static` items in const fn".into())),
269270
None => Ok(()),
270271
},
271272
}
272273
}
273274

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

277278
while let [ref proj_base @ .., elem] = *cursor {
278279
cursor = proj_base;
279280
match elem {
280-
ProjectionElem::Field(..) => {
281+
ProjectionElem::Field(_, ty) => {
282+
if !is_ty_const_destruct(tcx, ty, body) && in_move {
283+
return Err((
284+
span,
285+
"cannot drop locals with a non constant destructor in const fn".into(),
286+
));
287+
}
288+
281289
let base_ty = Place::ty_from(place.local, proj_base, body, tcx).ty;
282290
if let Some(def) = base_ty.ty_adt_def() {
283291
// No union field accesses in `const fn`
@@ -320,7 +328,7 @@ fn check_terminator<'tcx>(
320328
"cannot drop locals with a non constant destructor in const fn".into(),
321329
));
322330
}
323-
check_place(tcx, *place, span, body)
331+
check_place(tcx, *place, span, body, false)
324332
},
325333
TerminatorKind::SwitchInt { discr, targets: _ } => check_operand(tcx, discr, span, body),
326334
TerminatorKind::GeneratorDrop | TerminatorKind::Yield { .. } => {
@@ -408,6 +416,7 @@ fn is_const_fn(tcx: TyCtxt<'_>, def_id: DefId, msrv: &Msrv) -> bool {
408416
})
409417
}
410418

419+
#[expect(clippy::similar_names)] // bit too pedantic
411420
fn is_ty_const_destruct<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, body: &Body<'tcx>) -> bool {
412421
// Avoid selecting for simple cases, such as builtin types.
413422
if ty::util::is_trivially_const_drop(ty) {
@@ -417,23 +426,24 @@ fn is_ty_const_destruct<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, body: &Body<'tcx>
417426
let obligation = Obligation::new(
418427
tcx,
419428
ObligationCause::dummy_with_span(body.span),
420-
ConstCx::new(tcx, body).param_env.with_constness(Constness::Const),
429+
ConstCx::new(tcx, body).param_env.with_const(),
421430
TraitRef::from_lang_item(tcx, LangItem::Destruct, body.span, [ty]).with_constness(BoundConstness::ConstIfConst),
422431
);
423432

424-
let fields_all_const_destruct = if let ty::Adt(def, subst) = ty.kind() && !ty.is_union() {
425-
// This is such a mess even rustfmt doesn't wanna touch it
426-
def.all_fields()
427-
.map(|field| is_ty_const_destruct(tcx, field.ty(tcx, subst), body))
428-
.all(|f| f)
429-
&& def.variants().iter()
430-
.map(|variant| variant.fields.iter().map(|field| is_ty_const_destruct(tcx, field.ty(tcx, subst), body)))
431-
.all(|mut fs| fs.all(|f| f))
432-
} else {
433-
true
434-
};
435-
436433
let infcx = tcx.infer_ctxt().build();
437434
let mut selcx = SelectionContext::new(&infcx);
438-
selcx.select(&obligation).is_ok() && fields_all_const_destruct
435+
let Some(impl_src) = selcx.select(&obligation).ok().flatten() else {
436+
return false;
437+
};
438+
439+
if !matches!(
440+
impl_src,
441+
ImplSource::ConstDestruct(_) | ImplSource::Param(_, ty::BoundConstness::ConstIfConst)
442+
) {
443+
return false;
444+
}
445+
446+
let ocx = ObligationCtxt::new(&infcx);
447+
ocx.register_obligations(impl_src.nested_obligations());
448+
ocx.select_all_or_error().is_empty()
439449
}

tests/ui/missing_const_for_fn/cant_be_const.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,13 +127,33 @@ with_span! {
127127
fn dont_check_in_proc_macro() {}
128128
}
129129

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

132133
enum A {
133134
F(String),
134135
N,
135136
}
136137

138+
// Same here.
137139
fn b(this: A) {}
138140

141+
// Minimized version of `a`.
139142
fn c(this: Vec<u16>) {}
143+
144+
struct F(A);
145+
146+
// Do not lint
147+
fn f(this: F) {}
148+
149+
// Do not lint
150+
fn g<T>(this: T) {}
151+
152+
struct Issue10617(String);
153+
154+
impl Issue10617 {
155+
// Do not lint
156+
pub fn name(self) -> String {
157+
self.0
158+
}
159+
}

tests/ui/missing_const_for_fn/could_be_const.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#![warn(clippy::missing_const_for_fn)]
22
#![allow(incomplete_features, clippy::let_and_return)]
3+
#![feature(const_mut_refs)]
4+
#![feature(const_trait_impl)]
35

46
use std::mem::transmute;
57

@@ -87,3 +89,14 @@ fn msrv_1_46() -> i32 {
8789

8890
// Should not be const
8991
fn main() {}
92+
93+
struct D;
94+
95+
impl const Drop for D {
96+
fn drop(&mut self) {
97+
todo!();
98+
}
99+
}
100+
101+
// Lint this, since it can be dropped in const contexts
102+
fn d(this: D) {}
Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this could be a `const fn`
2-
--> $DIR/could_be_const.rs:12:5
2+
--> $DIR/could_be_const.rs:14:5
33
|
44
LL | / pub fn new() -> Self {
55
LL | | Self { guess: 42 }
@@ -9,23 +9,23 @@ LL | | }
99
= note: `-D clippy::missing-const-for-fn` implied by `-D warnings`
1010

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

1919
error: this could be a `const fn`
20-
--> $DIR/could_be_const.rs:22:1
20+
--> $DIR/could_be_const.rs:24:1
2121
|
2222
LL | / fn one() -> i32 {
2323
LL | | 1
2424
LL | | }
2525
| |_^
2626

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

3636
error: this could be a `const fn`
37-
--> $DIR/could_be_const.rs:33:1
37+
--> $DIR/could_be_const.rs:35:1
3838
|
3939
LL | / fn string() -> String {
4040
LL | | String::new()
4141
LL | | }
4242
| |_^
4343

4444
error: this could be a `const fn`
45-
--> $DIR/could_be_const.rs:38:1
45+
--> $DIR/could_be_const.rs:40:1
4646
|
4747
LL | / unsafe fn four() -> i32 {
4848
LL | | 4
4949
LL | | }
5050
| |_^
5151

5252
error: this could be a `const fn`
53-
--> $DIR/could_be_const.rs:43:1
53+
--> $DIR/could_be_const.rs:45:1
5454
|
5555
LL | / fn generic<T>(t: T) -> T {
5656
LL | | t
5757
LL | | }
5858
| |_^
5959

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

6868
error: this could be a `const fn`
69-
--> $DIR/could_be_const.rs:64:9
69+
--> $DIR/could_be_const.rs:66:9
7070
|
7171
LL | / pub fn b(self, a: &A) -> B {
7272
LL | | B
7373
LL | | }
7474
| |_________^
7575

7676
error: this could be a `const fn`
77-
--> $DIR/could_be_const.rs:73:5
77+
--> $DIR/could_be_const.rs:75:5
7878
|
7979
LL | / fn const_fn_stabilized_before_msrv(byte: u8) {
8080
LL | | byte.is_ascii_digit();
8181
LL | | }
8282
| |_____^
8383

8484
error: this could be a `const fn`
85-
--> $DIR/could_be_const.rs:84:1
85+
--> $DIR/could_be_const.rs:86:1
8686
|
8787
LL | / fn msrv_1_46() -> i32 {
8888
LL | | 46
8989
LL | | }
9090
| |_^
9191

92-
error: aborting due to 11 previous errors
92+
error: this could be a `const fn`
93+
--> $DIR/could_be_const.rs:102:1
94+
|
95+
LL | fn d(this: D) {}
96+
| ^^^^^^^^^^^^^^^^
97+
98+
error: aborting due to 12 previous errors
9399

0 commit comments

Comments
 (0)