Skip to content

Commit

Permalink
Run const_prop_lint in check builds, too
Browse files Browse the repository at this point in the history
  • Loading branch information
oli-obk committed Feb 20, 2024
1 parent 5af2130 commit 6add19d
Show file tree
Hide file tree
Showing 159 changed files with 2,308 additions and 1,234 deletions.
2 changes: 2 additions & 0 deletions compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
});
});
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/query/erase.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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> {
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_middle/src/traits/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Up @@ -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`.
Expand Down
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
Expand Up @@ -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()
Expand Down Expand Up @@ -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() {
Expand Down
83 changes: 65 additions & 18 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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 {
Expand All @@ -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.
//
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)));
}
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions src/doc/rustc/src/lints/levels.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
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
Expand Up @@ -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
Expand Down
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
|
Expand All @@ -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];
| ^^^^
Expand All @@ -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
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 6add19d

Please sign in to comment.