Skip to content
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

Run const_prop_lint in check builds, too #120687

Closed
wants to merge 1 commit into from
Closed
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
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