Skip to content

Run const_prop_lint in check builds, too #120687

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

Closed
wants to merge 1 commit into from
Closed
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
2 changes: 2 additions & 0 deletions compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
@@ -734,6 +734,8 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> {
// Run unsafety check because it's responsible for stealing and
// deallocating THIR.
tcx.ensure().check_unsafety(def_id);
tcx.ensure().const_prop_lint(def_id);
tcx.ensure().const_prop_lint_promoteds(def_id);
tcx.ensure().mir_borrowck(def_id)
});
});
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/mir/tcx.rs
Original file line number Diff line number Diff line change
@@ -76,6 +76,10 @@ impl<'tcx> PlaceTy<'tcx> {
if self.variant_index.is_some() && !matches!(elem, ProjectionElem::Field(..)) {
bug!("cannot use non field projection on downcasted place")
}
if self.ty.references_error() {
// Cannot do anything useful with error types
return PlaceTy::from_ty(self.ty);
}
let answer = match *elem {
ProjectionElem::Deref => {
let ty = self
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/query/erase.rs
Original file line number Diff line number Diff line change
@@ -243,6 +243,7 @@ trivial! {
Option<usize>,
Option<rustc_span::Symbol>,
Result<(), rustc_errors::ErrorGuaranteed>,
Result<(), traits::util::HasImpossiblePredicates>,
Result<(), rustc_middle::traits::query::NoSolution>,
Result<rustc_middle::traits::EvaluationResult, rustc_middle::traits::OverflowError>,
rustc_ast::expand::allocator::AllocatorKind,
13 changes: 13 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
@@ -40,6 +40,7 @@ use crate::traits::query::{
OutlivesBound,
};
use crate::traits::specialization_graph;
use crate::traits::util::HasImpossiblePredicates;
use crate::traits::{
CodegenObligationError, EvaluationResult, ImplSource, ObjectSafetyViolation, ObligationCause,
OverflowError, WellFormedLoc,
@@ -1015,6 +1016,18 @@ rustc_queries! {
cache_on_disk_if(tcx) { tcx.is_typeck_child(key.to_def_id()) }
}

/// Run the const prop lints on the `mir_promoted` of an item.
query const_prop_lint(key: LocalDefId) -> Result<(), HasImpossiblePredicates> {
desc { |tcx| "running const prop lints for `{}`", tcx.def_path_str(key) }
cache_on_disk_if { true }
}

/// Run the const prop lints on the promoted consts of an item.
query const_prop_lint_promoteds(key: LocalDefId) -> Result<(), HasImpossiblePredicates> {
desc { |tcx| "running const prop lints for promoteds of `{}`", tcx.def_path_str(key) }
cache_on_disk_if { true }
}

/// Gets a complete map from all types to their inherent impls.
/// Not meant to be used directly outside of coherence.
query crate_inherent_impls(k: ()) -> Result<&'tcx CrateInherentImpls, ErrorGuaranteed> {
6 changes: 6 additions & 0 deletions compiler/rustc_middle/src/traits/util.rs
Original file line number Diff line number Diff line change
@@ -60,3 +60,9 @@ impl<'tcx> Iterator for Elaborator<'tcx> {
}
}
}

/// Used as an error type to signal that an item may have an invalid body, because its
/// where bounds are trivially not satisfyable.
#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)]
#[derive(HashStable, Encodable, Decodable)]
pub struct HasImpossiblePredicates;
9 changes: 8 additions & 1 deletion compiler/rustc_middle/src/ty/util.rs
Original file line number Diff line number Diff line change
@@ -659,10 +659,17 @@ impl<'tcx> TyCtxt<'tcx> {
/// Get the type of the pointer to the static that we use in MIR.
pub fn static_ptr_ty(self, def_id: DefId) -> Ty<'tcx> {
// Make sure that any constants in the static's type are evaluated.
let static_ty = self.normalize_erasing_regions(
let mut static_ty = self.normalize_erasing_regions(
ty::ParamEnv::empty(),
self.type_of(def_id).instantiate_identity(),
);
if !static_ty.is_sized(self, ty::ParamEnv::reveal_all()) {
static_ty = Ty::new_error_with_message(
self,
self.def_span(def_id),
"unsized statics are forbidden and error in wfcheck",
);
}

// Make sure that accesses to unsafe statics end up using raw pointers.
// For thread-locals, this needs to be kept in sync with `Rvalue::ty`.
6 changes: 6 additions & 0 deletions compiler/rustc_mir_transform/src/const_prop_lint.rs
Original file line number Diff line number Diff line change
@@ -261,6 +261,11 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
// manually normalized.
let val = self.tcx.try_normalize_erasing_regions(self.param_env, c.const_).ok()?;

// Unsized constants are illegal and already errored in wfcheck
if !val.ty().is_sized(self.tcx, self.param_env) {
return None;
}

self.use_ecx(|this| this.ecx.eval_mir_constant(&val, Some(c.span), None))?
.as_mplace_or_imm()
.right()
@@ -471,6 +476,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
let value_const = self.use_ecx(|this| this.ecx.read_scalar(value))?;

if expected != value_const {
trace!("assertion failed");
// Poison all places this operand references so that further code
// doesn't use the invalid value
if let Some(place) = cond.place() {
83 changes: 65 additions & 18 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
@@ -38,6 +38,7 @@ use rustc_middle::mir::{
SourceInfo, Statement, StatementKind, TerminatorKind, START_BLOCK,
};
use rustc_middle::query::Providers;
use rustc_middle::traits::util::HasImpossiblePredicates;
use rustc_middle::ty::{self, TyCtxt, TypeVisitableExt};
use rustc_span::{source_map::Spanned, sym, DUMMY_SP};
use rustc_trait_selection::traits;
@@ -137,6 +138,8 @@ pub fn provide(providers: &mut Providers) {
is_ctfe_mir_available: |tcx, did| is_mir_available(tcx, did),
mir_callgraph_reachable: inline::cycle::mir_callgraph_reachable,
mir_inliner_callees: inline::cycle::mir_inliner_callees,
const_prop_lint,
const_prop_lint_promoteds,
promoted_mir,
deduced_param_attrs: deduce_param_attrs::deduced_param_attrs,
..*providers
@@ -393,6 +396,39 @@ fn inner_mir_for_ctfe(tcx: TyCtxt<'_>, def: LocalDefId) -> Body<'_> {
body
}

fn const_prop_lint(tcx: TyCtxt<'_>, def: LocalDefId) -> Result<(), HasImpossiblePredicates> {
let (body, _) = tcx.mir_promoted(def);
let body = body.borrow();

let mir_borrowck = tcx.mir_borrowck(def);

check_impossible_predicates(tcx, def)?;
if mir_borrowck.tainted_by_errors.is_none() && body.tainted_by_errors.is_none() {
const_prop_lint::ConstPropLint.run_lint(tcx, &body);
}
Ok(())
}

fn const_prop_lint_promoteds(
tcx: TyCtxt<'_>,
def: LocalDefId,
) -> Result<(), HasImpossiblePredicates> {
let (_, promoteds) = tcx.mir_promoted(def);
let promoteds = promoteds.borrow();

let mir_borrowck = tcx.mir_borrowck(def);

check_impossible_predicates(tcx, def)?;
if mir_borrowck.tainted_by_errors.is_none() {
for body in &*promoteds {
if body.tainted_by_errors.is_none() {
const_prop_lint::ConstPropLint.run_lint(tcx, &body);
}
}
}
Ok(())
}

/// Obtain just the main MIR (no promoteds) and run some cleanups on it. This also runs
/// mir borrowck *before* doing so in order to ensure that borrowck can be run and doesn't
/// end up missing the source MIR due to stealing happening.
@@ -401,6 +437,7 @@ fn mir_drops_elaborated_and_const_checked(tcx: TyCtxt<'_>, def: LocalDefId) -> &
tcx.ensure_with_value().mir_coroutine_witnesses(def);
}
let mir_borrowck = tcx.mir_borrowck(def);
let impossible_predicates = tcx.const_prop_lint(def);

let is_fn_like = tcx.def_kind(def).is_fn_like();
if is_fn_like {
@@ -415,7 +452,30 @@ fn mir_drops_elaborated_and_const_checked(tcx: TyCtxt<'_>, def: LocalDefId) -> &
if let Some(error_reported) = mir_borrowck.tainted_by_errors {
body.tainted_by_errors = Some(error_reported);
}
if impossible_predicates.is_err() {
trace!("found unsatisfiable predicates for {:?}", body.source);
// Clear the body to only contain a single `unreachable` statement.
let bbs = body.basic_blocks.as_mut();
bbs.raw.truncate(1);
bbs[START_BLOCK].statements.clear();
bbs[START_BLOCK].terminator_mut().kind = TerminatorKind::Unreachable;
body.var_debug_info.clear();
body.local_decls.raw.truncate(body.arg_count + 1);
}

run_analysis_to_runtime_passes(tcx, &mut body);

// Now that drop elaboration has been performed, we can check for
// unconditional drop recursion.
rustc_mir_build::lints::check_drop_recursion(tcx, &body);

tcx.alloc_steal_mir(body)
}

fn check_impossible_predicates(
tcx: TyCtxt<'_>,
def: LocalDefId,
) -> Result<(), HasImpossiblePredicates> {
// Check if it's even possible to satisfy the 'where' clauses
// for this item.
//
@@ -445,28 +505,15 @@ fn mir_drops_elaborated_and_const_checked(tcx: TyCtxt<'_>, def: LocalDefId) -> &
// the normalization code (leading to cycle errors), since
// it's usually never invoked in this way.
let predicates = tcx
.predicates_of(body.source.def_id())
.predicates_of(def)
.predicates
.iter()
.filter_map(|(p, _)| if p.is_global() { Some(*p) } else { None });
if traits::impossible_predicates(tcx, traits::elaborate(tcx, predicates).collect()) {
trace!("found unsatisfiable predicates for {:?}", body.source);
// Clear the body to only contain a single `unreachable` statement.
let bbs = body.basic_blocks.as_mut();
bbs.raw.truncate(1);
bbs[START_BLOCK].statements.clear();
bbs[START_BLOCK].terminator_mut().kind = TerminatorKind::Unreachable;
body.var_debug_info.clear();
body.local_decls.raw.truncate(body.arg_count + 1);
Err(HasImpossiblePredicates)
} else {
Ok(())
}

run_analysis_to_runtime_passes(tcx, &mut body);

// Now that drop elaboration has been performed, we can check for
// unconditional drop recursion.
rustc_mir_build::lints::check_drop_recursion(tcx, &body);

tcx.alloc_steal_mir(body)
}

// Made public such that `mir_drops_elaborated_and_const_checked` can be overridden
@@ -533,7 +580,6 @@ fn run_runtime_lowering_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
&elaborate_box_derefs::ElaborateBoxDerefs,
&coroutine::StateTransform,
&add_retag::AddRetag,
&Lint(const_prop_lint::ConstPropLint),
];
pm::run_passes_no_validate(tcx, body, passes, Some(MirPhase::Runtime(RuntimePhase::Initial)));
}
@@ -678,6 +724,7 @@ fn promoted_mir(tcx: TyCtxt<'_>, def: LocalDefId) -> &IndexVec<Promoted, Body<'_
}

tcx.ensure_with_value().mir_borrowck(def);
tcx.ensure().const_prop_lint_promoteds(def);
let mut promoted = tcx.mir_promoted(def).1.steal();

for body in &mut promoted {
4 changes: 2 additions & 2 deletions src/doc/rustc/src/lints/levels.md
Original file line number Diff line number Diff line change
@@ -71,7 +71,7 @@ level is capped via cap-lints.
A 'deny' lint produces an error if you violate it. For example, this code
runs into the `exceeding_bitshifts` lint.

```rust,no_run
```rust,compile_fail
fn main() {
100u8 << 10;
}
@@ -246,7 +246,7 @@ pub fn foo() {}
This is the maximum level for all lints. So for example, if we take our
code sample from the "deny" lint level above:

```rust,no_run
```rust,compile_fail
fn main() {
100u8 << 10;
}
5 changes: 2 additions & 3 deletions src/tools/clippy/tests/ui-toml/suppress_lint_in_const/test.rs
Original file line number Diff line number Diff line change
@@ -35,9 +35,8 @@ fn main() {
x[const { idx() }]; // Ok, should not produce stderr.
x[const { idx4() }]; // Ok, let rustc's `unconditional_panic` lint handle `usize` indexing on arrays.
const { &ARR[idx()] }; // Ok, should not produce stderr, since `suppress-restriction-lint-in-const` is set true.
const { &ARR[idx4()] }; // Ok, should not produce stderr, since `suppress-restriction-lint-in-const` is set true.
//
//~^^ ERROR: failed
// FIXME errors in rustc and subsequently blocks all lints in this file. Can reenable once solved on the rustc side
//const { &ARR[idx4()] }; // Ok, should not produce stderr, since `suppress-restriction-lint-in-const` is set true.

let y = &x;
y[0]; // Ok, referencing shouldn't affect this lint. See the issue 6021
24 changes: 6 additions & 18 deletions src/tools/clippy/tests/ui-toml/suppress_lint_in_const/test.stderr
Original file line number Diff line number Diff line change
@@ -1,15 +1,3 @@
error[E0080]: evaluation of `main::{constant#3}` failed
--> $DIR/test.rs:38:14
|
LL | const { &ARR[idx4()] }; // Ok, should not produce stderr, since `suppress-restriction-lint-in-const` is set true.
| ^^^^^^^^^^^ index out of bounds: the length is 2 but the index is 4

note: erroneous constant encountered
--> $DIR/test.rs:38:5
|
LL | const { &ARR[idx4()] }; // Ok, should not produce stderr, since `suppress-restriction-lint-in-const` is set true.
| ^^^^^^^^^^^^^^^^^^^^^^

error: indexing may panic
--> $DIR/test.rs:29:5
|
@@ -21,39 +9,39 @@ LL | x[index];
= help: to override `-D warnings` add `#[allow(clippy::indexing_slicing)]`

error: indexing may panic
--> $DIR/test.rs:47:5
--> $DIR/test.rs:46:5
|
LL | v[0];
| ^^^^
|
= help: consider using `.get(n)` or `.get_mut(n)` instead

error: indexing may panic
--> $DIR/test.rs:48:5
--> $DIR/test.rs:47:5
|
LL | v[10];
| ^^^^^
|
= help: consider using `.get(n)` or `.get_mut(n)` instead

error: indexing may panic
--> $DIR/test.rs:49:5
--> $DIR/test.rs:48:5
|
LL | v[1 << 3];
| ^^^^^^^^^
|
= help: consider using `.get(n)` or `.get_mut(n)` instead

error: indexing may panic
--> $DIR/test.rs:55:5
--> $DIR/test.rs:54:5
|
LL | v[N];
| ^^^^
|
= help: consider using `.get(n)` or `.get_mut(n)` instead

error: indexing may panic
--> $DIR/test.rs:56:5
--> $DIR/test.rs:55:5
|
LL | v[M];
| ^^^^
@@ -66,6 +54,6 @@ error[E0080]: evaluation of constant value failed
LL | const REF_ERR: &i32 = &ARR[idx4()]; // Ok, let rustc handle const contexts.
| ^^^^^^^^^^^ index out of bounds: the length is 2 but the index is 4

error: aborting due to 8 previous errors
error: aborting due to 7 previous errors

For more information about this error, try `rustc --explain E0080`.
4 changes: 2 additions & 2 deletions src/tools/clippy/tests/ui/indexing_slicing_index.rs
Original file line number Diff line number Diff line change
@@ -45,8 +45,8 @@ fn main() {
const { &ARR[idx()] };
//~^ ERROR: indexing may panic
// This should be linted, since `suppress-restriction-lint-in-const` default is false.
const { &ARR[idx4()] };
//~^ ERROR: indexing may panic
// FIXME can't include here for now, as the error on it causes all lints to get silenced
//const { &ARR[idx4()] };

let y = &x;
// Ok, referencing shouldn't affect this lint. See the issue 6021
Loading