Skip to content

Commit

Permalink
Auto merge of #125824 - matthiaskrgr:rollup-eam7i0p, r=matthiaskrgr
Browse files Browse the repository at this point in the history
Rollup of 7 pull requests

Successful merges:

 - #125652 (Revert propagation of drop-live information from Polonius)
 - #125730 (Apply `x clippy --fix` and `x fmt` on Rustc)
 - #125756 (coverage: Optionally instrument the RHS of lazy logical operators)
 - #125776 (Stop using `translate_args` in the new solver)
 - #125796 (Also InstSimplify `&raw*`)
 - #125807 (Also resolve the type of constants, even if we already turned it into an error constant)
 - #125816 (Don't build the `rust-demangler` binary for coverage tests)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed May 31, 2024
2 parents 2a2c29a + df9cd17 commit ada5e2c
Show file tree
Hide file tree
Showing 51 changed files with 876 additions and 162 deletions.
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
1 change: 1 addition & 0 deletions compiler/rustc_hir_typeck/src/writeback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -865,6 +865,7 @@ impl<'cx, 'tcx> TypeFolder<TyCtxt<'tcx>> for Resolver<'cx, 'tcx> {
self.handle_term(ct, ty::Const::outer_exclusive_binder, |tcx, guar| {
ty::Const::new_error(tcx, guar, ct.ty())
})
.super_fold_with(self)
}

fn fold_predicate(&mut self, predicate: ty::Predicate<'tcx>) -> ty::Predicate<'tcx> {
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

0 comments on commit ada5e2c

Please sign in to comment.