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

Rollup of 5 pull requests #125812

Closed
wants to merge 12 commits 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
11 changes: 3 additions & 8 deletions compiler/rustc_borrowck/src/type_check/liveness/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use std::rc::Rc;
use crate::{
constraints::OutlivesConstraintSet,
facts::{AllFacts, AllFactsExt},
location::LocationTable,
region_infer::values::LivenessValues,
universal_regions::UniversalRegions,
};
Expand All @@ -39,7 +38,6 @@ pub(super) fn generate<'mir, 'tcx>(
elements: &Rc<DenseLocationMap>,
flow_inits: &mut ResultsCursor<'mir, 'tcx, MaybeInitializedPlaces<'mir, 'tcx>>,
move_data: &MoveData<'tcx>,
location_table: &LocationTable,
use_polonius: bool,
) {
debug!("liveness::generate");
Expand All @@ -53,11 +51,9 @@ pub(super) fn generate<'mir, 'tcx>(
compute_relevant_live_locals(typeck.tcx(), &free_regions, body);
let facts_enabled = use_polonius || AllFacts::enabled(typeck.tcx());

let polonius_drop_used = facts_enabled.then(|| {
let mut drop_used = Vec::new();
polonius::populate_access_facts(typeck, body, location_table, move_data, &mut drop_used);
drop_used
});
if facts_enabled {
polonius::populate_access_facts(typeck, body, move_data);
};

trace::trace(
typeck,
Expand All @@ -67,7 +63,6 @@ pub(super) fn generate<'mir, 'tcx>(
move_data,
relevant_live_locals,
boring_locals,
polonius_drop_used,
);

// Mark regions that should be live where they appear within rvalues or within a call: like
Expand Down
9 changes: 1 addition & 8 deletions compiler/rustc_borrowck/src/type_check/liveness/polonius.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,10 @@ impl<'a, 'tcx> Visitor<'tcx> for UseFactsExtractor<'a, 'tcx> {
pub(super) fn populate_access_facts<'a, 'tcx>(
typeck: &mut TypeChecker<'a, 'tcx>,
body: &Body<'tcx>,
location_table: &LocationTable,
move_data: &MoveData<'tcx>,
//FIXME: this is not mutated, but expected to be modified as
// out param, bug?
dropped_at: &mut Vec<(Local, Location)>,
) {
debug!("populate_access_facts()");
let location_table = typeck.borrowck_context.location_table;

if let Some(facts) = typeck.borrowck_context.all_facts.as_mut() {
let mut extractor = UseFactsExtractor {
Expand All @@ -104,10 +101,6 @@ pub(super) fn populate_access_facts<'a, 'tcx>(
};
extractor.visit_body(body);

facts.var_dropped_at.extend(
dropped_at.iter().map(|&(local, location)| (local, location_table.mid_index(location))),
);

for (local, local_decl) in body.local_decls.iter_enumerated() {
debug!(
"add use_of_var_derefs_origin facts - local={:?}, type={:?}",
Expand Down
35 changes: 25 additions & 10 deletions compiler/rustc_borrowck/src/type_check/liveness/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use rustc_mir_dataflow::impls::MaybeInitializedPlaces;
use rustc_mir_dataflow::move_paths::{HasMoveData, MoveData, MovePathIndex};
use rustc_mir_dataflow::ResultsCursor;

use crate::location::RichLocation;
use crate::{
region_infer::values::{self, LiveLoans},
type_check::liveness::local_use_map::LocalUseMap,
Expand Down Expand Up @@ -46,7 +47,6 @@ pub(super) fn trace<'mir, 'tcx>(
move_data: &MoveData<'tcx>,
relevant_live_locals: Vec<Local>,
boring_locals: Vec<Local>,
polonius_drop_used: Option<Vec<(Local, Location)>>,
) {
let local_use_map = &LocalUseMap::build(&relevant_live_locals, elements, body);

Expand Down Expand Up @@ -93,9 +93,7 @@ pub(super) fn trace<'mir, 'tcx>(

let mut results = LivenessResults::new(cx);

if let Some(drop_used) = polonius_drop_used {
results.add_extra_drop_facts(drop_used, relevant_live_locals.iter().copied().collect())
}
results.add_extra_drop_facts(&relevant_live_locals);

results.compute_for_all_locals(relevant_live_locals);

Expand Down Expand Up @@ -218,21 +216,38 @@ impl<'me, 'typeck, 'flow, 'tcx> LivenessResults<'me, 'typeck, 'flow, 'tcx> {
///
/// Add facts for all locals with free regions, since regions may outlive
/// the function body only at certain nodes in the CFG.
fn add_extra_drop_facts(
&mut self,
drop_used: Vec<(Local, Location)>,
relevant_live_locals: FxIndexSet<Local>,
) {
fn add_extra_drop_facts(&mut self, relevant_live_locals: &[Local]) -> Option<()> {
let drop_used = self
.cx
.typeck
.borrowck_context
.all_facts
.as_ref()
.map(|facts| facts.var_dropped_at.clone())?;

let relevant_live_locals: FxIndexSet<_> = relevant_live_locals.iter().copied().collect();

let locations = IntervalSet::new(self.cx.elements.num_points());

for (local, location) in drop_used {
for (local, location_index) in drop_used {
if !relevant_live_locals.contains(&local) {
let local_ty = self.cx.body.local_decls[local].ty;
if local_ty.has_free_regions() {
let location = match self
.cx
.typeck
.borrowck_context
.location_table
.to_location(location_index)
{
RichLocation::Start(l) => l,
RichLocation::Mid(l) => l,
};
self.cx.add_drop_live_facts_for(local, local_ty, &[location], &locations);
}
}
}
Some(())
}

/// Clear the value of fields that are "per local variable".
Expand Down
10 changes: 1 addition & 9 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,15 +188,7 @@ pub(crate) fn type_check<'mir, 'tcx>(
checker.equate_inputs_and_outputs(body, universal_regions, &normalized_inputs_and_output);
checker.check_signature_annotation(body);

liveness::generate(
&mut checker,
body,
elements,
flow_inits,
move_data,
location_table,
use_polonius,
);
liveness::generate(&mut checker, body, elements, flow_inits, move_data, use_polonius);

translate_outlives_facts(&mut checker);
let opaque_type_values = infcx.take_opaque_types();
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_data_structures/src/graph/dominators/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ fn dominators_impl<G: ControlFlowGraph>(graph: &G) -> Inner<G::Node> {
// These are all done here rather than through one of the 'standard'
// graph traversals to help make this fast.
'recurse: while let Some(frame) = stack.last_mut() {
while let Some(successor) = frame.iter.next() {
for successor in frame.iter.by_ref() {
if real_to_pre_order[successor].is_none() {
let pre_order_idx = pre_order_to_real.push(successor);
real_to_pre_order[successor] = Some(pre_order_idx);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_data_structures/src/graph/iterate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ fn post_order_walk<G: DirectedGraph + Successors>(
let node = frame.node;
visited[node] = true;

while let Some(successor) = frame.iter.next() {
for successor in frame.iter.by_ref() {
if !visited[successor] {
stack.push(PostOrderFrame { node: successor, iter: graph.successors(successor) });
continue 'recurse;
Expand Down Expand Up @@ -112,7 +112,7 @@ where
/// This is equivalent to just invoke `next` repeatedly until
/// you get a `None` result.
pub fn complete_search(&mut self) {
while let Some(_) = self.next() {}
for _ in self.by_ref() {}
}

/// Returns true if node has been visited thus far.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_data_structures/src/graph/scc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub struct SccData<S: Idx> {
}

impl<N: Idx, S: Idx + Ord> Sccs<N, S> {
pub fn new(graph: &(impl DirectedGraph<Node = N> + Successors)) -> Self {
pub fn new(graph: &impl Successors<Node = N>) -> Self {
SccsConstruction::construct(graph)
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_data_structures/src/profiling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ impl SelfProfiler {
// ASLR is disabled and the heap is otherwise deterministic.
let pid: u32 = process::id();
let filename = format!("{crate_name}-{pid:07}.rustc_profile");
let path = output_directory.join(&filename);
let path = output_directory.join(filename);
let profiler =
Profiler::with_counter(&path, measureme::counters::Counter::by_name(counter_name)?)?;

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_data_structures/src/sorted_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,13 @@ impl<K: Ord, V> SortedMap<K, V> {

/// Iterate over the keys, sorted
#[inline]
pub fn keys(&self) -> impl Iterator<Item = &K> + ExactSizeIterator + DoubleEndedIterator {
pub fn keys(&self) -> impl ExactSizeIterator<Item = &K> + DoubleEndedIterator {
self.data.iter().map(|(k, _)| k)
}

/// Iterate over values, sorted by key
#[inline]
pub fn values(&self) -> impl Iterator<Item = &V> + ExactSizeIterator + DoubleEndedIterator {
pub fn values(&self) -> impl ExactSizeIterator<Item = &V> + DoubleEndedIterator {
self.data.iter().map(|(_, v)| v)
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_data_structures/src/sync/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ mod maybe_sync {
match self.mode {
Mode::NoSync => {
let cell = unsafe { &self.lock.mode_union.no_sync };
debug_assert_eq!(cell.get(), true);
debug_assert!(cell.get());
cell.set(false);
}
// SAFETY (unlock): We know that the lock is locked as this type is a proof of that.
Expand Down
57 changes: 57 additions & 0 deletions compiler/rustc_mir_build/src/build/coverageinfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,63 @@ impl BranchInfoBuilder {
}

impl<'tcx> Builder<'_, 'tcx> {
/// If condition coverage is enabled, inject extra blocks and marker statements
/// that will let us track the value of the condition in `place`.
pub(crate) fn visit_coverage_standalone_condition(
&mut self,
mut expr_id: ExprId, // Expression giving the span of the condition
place: mir::Place<'tcx>, // Already holds the boolean condition value
block: &mut BasicBlock,
) {
// Bail out if condition coverage is not enabled for this function.
let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };
if !self.tcx.sess.instrument_coverage_condition() {
return;
};

// Remove any wrappers, so that we can inspect the real underlying expression.
while let ExprKind::Use { source: inner } | ExprKind::Scope { value: inner, .. } =
self.thir[expr_id].kind
{
expr_id = inner;
}
// If the expression is a lazy logical op, it will naturally get branch
// coverage as part of its normal lowering, so we can disregard it here.
if let ExprKind::LogicalOp { .. } = self.thir[expr_id].kind {
return;
}

let source_info = SourceInfo { span: self.thir[expr_id].span, scope: self.source_scope };

// Using the boolean value that has already been stored in `place`, set up
// control flow in the shape of a diamond, so that we can place separate
// marker statements in the true and false blocks. The coverage MIR pass
// will use those markers to inject coverage counters as appropriate.
//
// block
// / \
// true_block false_block
// (marker) (marker)
// \ /
// join_block

let true_block = self.cfg.start_new_block();
let false_block = self.cfg.start_new_block();
self.cfg.terminate(
*block,
source_info,
mir::TerminatorKind::if_(mir::Operand::Copy(place), true_block, false_block),
);

branch_info.add_two_way_branch(&mut self.cfg, source_info, true_block, false_block);

let join_block = self.cfg.start_new_block();
self.cfg.goto(true_block, source_info, join_block);
self.cfg.goto(false_block, source_info, join_block);
// Any subsequent codegen in the caller should use the new join block.
*block = join_block;
}

/// If branch coverage is enabled, inject marker statements into `then_block`
/// and `else_block`, and record their IDs in the table of branch spans.
pub(crate) fn visit_coverage_branch_condition(
Expand Down
8 changes: 6 additions & 2 deletions compiler/rustc_mir_build/src/build/expr/into.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
const_: Const::from_bool(this.tcx, constant),
},
);
let rhs = unpack!(this.expr_into_dest(destination, continuation, rhs));
let mut rhs_block = unpack!(this.expr_into_dest(destination, continuation, rhs));
// Instrument the lowered RHS's value for condition coverage.
// (Does nothing if condition coverage is not enabled.)
this.visit_coverage_standalone_condition(rhs, destination, &mut rhs_block);

let target = this.cfg.start_new_block();
this.cfg.goto(rhs, source_info, target);
this.cfg.goto(rhs_block, source_info, target);
this.cfg.goto(short_circuit, source_info, target);
target.unit()
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/instsimplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ impl<'tcx> InstSimplifyContext<'tcx, '_> {

/// Transform `&(*a)` ==> `a`.
fn simplify_ref_deref(&self, source_info: &SourceInfo, rvalue: &mut Rvalue<'tcx>) {
if let Rvalue::Ref(_, _, place) = rvalue {
if let Rvalue::Ref(_, _, place) | Rvalue::AddressOf(_, place) = rvalue {
if let Some((base, ProjectionElem::Deref)) = place.as_ref().last_projection() {
if rvalue.ty(self.local_decls, self.tcx) != base.ty(self.local_decls, self.tcx).ty {
return;
Expand Down
18 changes: 8 additions & 10 deletions compiler/rustc_parse_format/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,13 +286,11 @@ impl<'a> Iterator for Parser<'a> {
lbrace_byte_pos.to(InnerOffset(rbrace_byte_pos.0 + width)),
);
}
} else {
if let Some(&(_, maybe)) = self.cur.peek() {
match maybe {
'?' => self.suggest_format_debug(),
'<' | '^' | '>' => self.suggest_format_align(maybe),
_ => self.suggest_positional_arg_instead_of_captured_arg(arg),
}
} else if let Some(&(_, maybe)) = self.cur.peek() {
match maybe {
'?' => self.suggest_format_debug(),
'<' | '^' | '>' => self.suggest_format_align(maybe),
_ => self.suggest_positional_arg_instead_of_captured_arg(arg),
}
}
Some(NextArgument(Box::new(arg)))
Expand Down Expand Up @@ -1028,7 +1026,7 @@ fn find_width_map_from_snippet(
if next_c == '{' {
// consume up to 6 hexanumeric chars
let digits_len =
s.clone().take(6).take_while(|(_, c)| c.is_digit(16)).count();
s.clone().take(6).take_while(|(_, c)| c.is_ascii_hexdigit()).count();

let len_utf8 = s
.as_str()
Expand All @@ -1047,14 +1045,14 @@ fn find_width_map_from_snippet(
width += required_skips + 2;

s.nth(digits_len);
} else if next_c.is_digit(16) {
} else if next_c.is_ascii_hexdigit() {
width += 1;

// We suggest adding `{` and `}` when appropriate, accept it here as if
// it were correct
let mut i = 0; // consume up to 6 hexanumeric chars
while let (Some((_, c)), _) = (s.next(), i < 6) {
if c.is_digit(16) {
if c.is_ascii_hexdigit() {
width += 1;
} else {
break;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_serialize/src/serialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ impl<S: Encoder> Encodable<S> for () {
}

impl<D: Decoder> Decodable<D> for () {
fn decode(_: &mut D) -> () {}
fn decode(_: &mut D) {}
}

impl<S: Encoder, T> Encodable<S> for PhantomData<T> {
Expand Down
18 changes: 17 additions & 1 deletion compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,23 @@ pub enum CoverageLevel {
Block,
/// Also instrument branch points (includes block coverage).
Branch,
/// Instrument for MC/DC. Mostly a superset of branch coverage, but might
/// Same as branch coverage, but also adds branch instrumentation for
/// certain boolean expressions that are not directly used for branching.
///
/// For example, in the following code, `b` does not directly participate
/// in a branch, but condition coverage will instrument it as its own
/// artificial branch:
/// ```
/// # let (a, b) = (false, true);
/// let x = a && b;
/// // ^ last operand
/// ```
///
/// This level is mainly intended to be a stepping-stone towards full MC/DC
/// instrumentation, so it might be removed in the future when MC/DC is
/// sufficiently complete, or if it is making MC/DC changes difficult.
Condition,
/// Instrument for MC/DC. Mostly a superset of condition coverage, but might
/// differ in some corner cases.
Mcdc,
}
Expand Down
Loading
Loading