Skip to content

Commit

Permalink
Auto merge of rust-lang#85720 - Dylan-DPC:rollup-in5917x, r=Dylan-DPC
Browse files Browse the repository at this point in the history
Rollup of 8 pull requests

Successful merges:

 - rust-lang#85478 (Disallow shadowing const parameters)
 - rust-lang#85625 (Prevent double drop in `Vec::dedup_by` if a destructor panics)
 - rust-lang#85627 (Fix a few details in THIR unsafeck)
 - rust-lang#85633 (Post-monomorphization errors traces MVP)
 - rust-lang#85670 (Remove arrays/IntoIterator message from Iterator trait.)
 - rust-lang#85678 (fix `matches!` and `assert_matches!` on edition 2021)
 - rust-lang#85679 (Remove num_as_ne_bytes feature)
 - rust-lang#85712 (Fix typo in core::array::IntoIter comment)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed May 26, 2021
2 parents 1969c2e + 9ee87c7 commit f6a28aa
Show file tree
Hide file tree
Showing 27 changed files with 444 additions and 201 deletions.
9 changes: 9 additions & 0 deletions compiler/rustc_middle/src/mir/mono.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,15 @@ impl<'tcx> MonoItem<'tcx> {
pub fn codegen_dep_node(&self, tcx: TyCtxt<'tcx>) -> DepNode {
crate::dep_graph::make_compile_mono_item(tcx, self)
}

/// Returns the item's `CrateNum`
pub fn krate(&self) -> CrateNum {
match self {
MonoItem::Fn(ref instance) => instance.def_id().krate,
MonoItem::Static(def_id) => def_id.krate,
MonoItem::GlobalAsm(..) => LOCAL_CRATE,
}
}
}

impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for MonoItem<'tcx> {
Expand Down
46 changes: 44 additions & 2 deletions compiler/rustc_mir/src/monomorphize/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::sync::{par_iter, MTLock, MTRef, ParallelIterator};
use rustc_errors::{ErrorReported, FatalError};
use rustc_hir as hir;
use rustc_hir::def_id::{DefId, DefIdMap, LocalDefId};
use rustc_hir::def_id::{DefId, DefIdMap, LocalDefId, LOCAL_CRATE};
use rustc_hir::itemlikevisit::ItemLikeVisitor;
use rustc_hir::lang_items::LangItem;
use rustc_index::bit_set::GrowableBitSet;
Expand Down Expand Up @@ -342,7 +342,8 @@ fn collect_roots(tcx: TyCtxt<'_>, mode: MonoItemCollectionMode) -> Vec<MonoItem<
.collect()
}

// Collect all monomorphized items reachable from `starting_point`
/// Collect all monomorphized items reachable from `starting_point`, and emit a note diagnostic if a
/// post-monorphization error is encountered during a collection step.
fn collect_items_rec<'tcx>(
tcx: TyCtxt<'tcx>,
starting_point: Spanned<MonoItem<'tcx>>,
Expand All @@ -359,6 +360,31 @@ fn collect_items_rec<'tcx>(
let mut neighbors = Vec::new();
let recursion_depth_reset;

//
// Post-monomorphization errors MVP
//
// We can encounter errors while monomorphizing an item, but we don't have a good way of
// showing a complete stack of spans ultimately leading to collecting the erroneous one yet.
// (It's also currently unclear exactly which diagnostics and information would be interesting
// to report in such cases)
//
// This leads to suboptimal error reporting: a post-monomorphization error (PME) will be
// shown with just a spanned piece of code causing the error, without information on where
// it was called from. This is especially obscure if the erroneous mono item is in a
// dependency. See for example issue #85155, where, before minimization, a PME happened two
// crates downstream from libcore's stdarch, without a way to know which dependency was the
// cause.
//
// If such an error occurs in the current crate, its span will be enough to locate the
// source. If the cause is in another crate, the goal here is to quickly locate which mono
// item in the current crate is ultimately responsible for causing the error.
//
// To give at least _some_ context to the user: while collecting mono items, we check the
// error count. If it has changed, a PME occurred, and we trigger some diagnostics about the
// current step of mono items collection.
//
let error_count = tcx.sess.diagnostic().err_count();

match starting_point.node {
MonoItem::Static(def_id) => {
let instance = Instance::mono(tcx, def_id);
Expand Down Expand Up @@ -411,6 +437,22 @@ fn collect_items_rec<'tcx>(
}
}

// Check for PMEs and emit a diagnostic if one happened. To try to show relevant edges of the
// mono item graph where the PME diagnostics are currently the most problematic (e.g. ones
// involving a dependency, and the lack of context is confusing) in this MVP, we focus on
// diagnostics on edges crossing a crate boundary: the collected mono items which are not
// defined in the local crate.
if tcx.sess.diagnostic().err_count() > error_count && starting_point.node.krate() != LOCAL_CRATE
{
tcx.sess.span_note_without_error(
starting_point.span,
&format!(
"the above error was encountered while instantiating `{}`",
starting_point.node
),
);
}

record_accesses(tcx, starting_point.node, neighbors.iter().map(|i| &i.node), inlining_map);

for neighbour in neighbors {
Expand Down
41 changes: 25 additions & 16 deletions compiler/rustc_mir_build/src/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
self.warn_unused_unsafe(
hir_id,
block_span,
Some(self.tcx.sess.source_map().guess_head_span(enclosing_span)),
Some((self.tcx.sess.source_map().guess_head_span(enclosing_span), "block")),
);
f(self);
} else {
Expand All @@ -52,7 +52,15 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
f(self);

if let SafetyContext::UnsafeBlock { used: false, span, hir_id } = self.safety_context {
self.warn_unused_unsafe(hir_id, span, self.body_unsafety.unsafe_fn_sig_span());
self.warn_unused_unsafe(
hir_id,
span,
if self.unsafe_op_in_unsafe_fn_allowed() {
self.body_unsafety.unsafe_fn_sig_span().map(|span| (span, "fn"))
} else {
None
},
);
}
self.safety_context = prev_context;
return;
Expand All @@ -72,16 +80,20 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
SafetyContext::UnsafeFn if unsafe_op_in_unsafe_fn_allowed => {}
SafetyContext::UnsafeFn => {
// unsafe_op_in_unsafe_fn is disallowed
struct_span_err!(
self.tcx.sess,
self.tcx.struct_span_lint_hir(
UNSAFE_OP_IN_UNSAFE_FN,
self.hir_context,
span,
E0133,
"{} is unsafe and requires unsafe block",
description,
|lint| {
lint.build(&format!(
"{} is unsafe and requires unsafe block (error E0133)",
description,
))
.span_label(span, description)
.note(note)
.emit();
},
)
.span_label(span, description)
.note(note)
.emit();
}
SafetyContext::Safe => {
let fn_sugg = if unsafe_op_in_unsafe_fn_allowed { " function or" } else { "" };
Expand All @@ -104,18 +116,15 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
&self,
hir_id: hir::HirId,
block_span: Span,
enclosing_span: Option<Span>,
enclosing_unsafe: Option<(Span, &'static str)>,
) {
let block_span = self.tcx.sess.source_map().guess_head_span(block_span);
self.tcx.struct_span_lint_hir(UNUSED_UNSAFE, hir_id, block_span, |lint| {
let msg = "unnecessary `unsafe` block";
let mut db = lint.build(msg);
db.span_label(block_span, msg);
if let Some(enclosing_span) = enclosing_span {
db.span_label(
enclosing_span,
format!("because it's nested under this `unsafe` block"),
);
if let Some((span, kind)) = enclosing_unsafe {
db.span_label(span, format!("because it's nested under this `unsafe` {}", kind));
}
db.emit();
});
Expand Down
23 changes: 14 additions & 9 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,24 +425,29 @@ impl<'a> Resolver<'a> {
}
err
}
ResolutionError::BindingShadowsSomethingUnacceptable(what_binding, name, binding) => {
let res = binding.res();
let shadows_what = res.descr();
ResolutionError::BindingShadowsSomethingUnacceptable {
shadowing_binding_descr,
name,
participle,
article,
shadowed_binding_descr,
shadowed_binding_span,
} => {
let mut err = struct_span_err!(
self.session,
span,
E0530,
"{}s cannot shadow {}s",
what_binding,
shadows_what
shadowing_binding_descr,
shadowed_binding_descr,
);
err.span_label(
span,
format!("cannot be named the same as {} {}", res.article(), shadows_what),
format!("cannot be named the same as {} {}", article, shadowed_binding_descr),
);
let participle = if binding.is_import() { "imported" } else { "defined" };
let msg = format!("the {} `{}` is {} here", shadows_what, name, participle);
err.span_label(binding.span, msg);
let msg =
format!("the {} `{}` is {} here", shadowed_binding_descr, name, participle);
err.span_label(shadowed_binding_span, msg);
err
}
ResolutionError::ForwardDeclaredTyParam => {
Expand Down
30 changes: 25 additions & 5 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1763,13 +1763,33 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
// to something unusable as a pattern (e.g., constructor function),
// but we still conservatively report an error, see
// issues/33118#issuecomment-233962221 for one reason why.
let binding = binding.expect("no binding for a ctor or static");
self.report_error(
ident.span,
ResolutionError::BindingShadowsSomethingUnacceptable(
pat_src.descr(),
ident.name,
binding.expect("no binding for a ctor or static"),
),
ResolutionError::BindingShadowsSomethingUnacceptable {
shadowing_binding_descr: pat_src.descr(),
name: ident.name,
participle: if binding.is_import() { "imported" } else { "defined" },
article: binding.res().article(),
shadowed_binding_descr: binding.res().descr(),
shadowed_binding_span: binding.span,
},
);
None
}
Res::Def(DefKind::ConstParam, def_id) => {
// Same as for DefKind::Const above, but here, `binding` is `None`, so we
// have to construct the error differently
self.report_error(
ident.span,
ResolutionError::BindingShadowsSomethingUnacceptable {
shadowing_binding_descr: pat_src.descr(),
name: ident.name,
participle: "defined",
article: res.article(),
shadowed_binding_descr: res.descr(),
shadowed_binding_span: self.r.opt_span(def_id).expect("const parameter defined outside of local crate"),
}
);
None
}
Expand Down
9 changes: 8 additions & 1 deletion compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,14 @@ enum ResolutionError<'a> {
/* current */ &'static str,
),
/// Error E0530: `X` bindings cannot shadow `Y`s.
BindingShadowsSomethingUnacceptable(&'static str, Symbol, &'a NameBinding<'a>),
BindingShadowsSomethingUnacceptable {
shadowing_binding_descr: &'static str,
name: Symbol,
participle: &'static str,
article: &'static str,
shadowed_binding_descr: &'static str,
shadowed_binding_span: Span,
},
/// Error E0128: generic parameters with a default cannot use forward-declared identifiers.
ForwardDeclaredTyParam, // FIXME(const_generics_defaults)
/// ERROR E0770: the type of const parameters must not depend on other generic parameters.
Expand Down
5 changes: 3 additions & 2 deletions library/alloc/src/vec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1619,6 +1619,8 @@ impl<T, A: Allocator> Vec<T, A> {
let prev_ptr = ptr.add(gap.write.wrapping_sub(1));

if same_bucket(&mut *read_ptr, &mut *prev_ptr) {
// Increase `gap.read` now since the drop may panic.
gap.read += 1;
/* We have found duplicate, drop it in-place */
ptr::drop_in_place(read_ptr);
} else {
Expand All @@ -1631,9 +1633,8 @@ impl<T, A: Allocator> Vec<T, A> {

/* We have filled that place, so go further */
gap.write += 1;
gap.read += 1;
}

gap.read += 1;
}

/* Technically we could let `gap` clean up with its Drop, but
Expand Down
48 changes: 25 additions & 23 deletions library/alloc/tests/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2234,48 +2234,50 @@ fn test_vec_dedup() {
#[test]
fn test_vec_dedup_panicking() {
#[derive(Debug)]
struct Panic {
drop_counter: &'static AtomicU32,
struct Panic<'a> {
drop_counter: &'a Cell<u32>,
value: bool,
index: usize,
}

impl PartialEq for Panic {
impl<'a> PartialEq for Panic<'a> {
fn eq(&self, other: &Self) -> bool {
self.value == other.value
}
}

impl Drop for Panic {
impl<'a> Drop for Panic<'a> {
fn drop(&mut self) {
let x = self.drop_counter.fetch_add(1, Ordering::SeqCst);
assert!(x != 4);
self.drop_counter.set(self.drop_counter.get() + 1);
if !std::thread::panicking() {
assert!(self.index != 4);
}
}
}

static DROP_COUNTER: AtomicU32 = AtomicU32::new(0);
let drop_counter = &Cell::new(0);
let expected = [
Panic { drop_counter: &DROP_COUNTER, value: false, index: 0 },
Panic { drop_counter: &DROP_COUNTER, value: false, index: 5 },
Panic { drop_counter: &DROP_COUNTER, value: true, index: 6 },
Panic { drop_counter: &DROP_COUNTER, value: true, index: 7 },
Panic { drop_counter, value: false, index: 0 },
Panic { drop_counter, value: false, index: 5 },
Panic { drop_counter, value: true, index: 6 },
Panic { drop_counter, value: true, index: 7 },
];
let mut vec = vec![
Panic { drop_counter: &DROP_COUNTER, value: false, index: 0 },
Panic { drop_counter, value: false, index: 0 },
// these elements get deduplicated
Panic { drop_counter: &DROP_COUNTER, value: false, index: 1 },
Panic { drop_counter: &DROP_COUNTER, value: false, index: 2 },
Panic { drop_counter: &DROP_COUNTER, value: false, index: 3 },
Panic { drop_counter: &DROP_COUNTER, value: false, index: 4 },
// here it panics
Panic { drop_counter: &DROP_COUNTER, value: false, index: 5 },
Panic { drop_counter: &DROP_COUNTER, value: true, index: 6 },
Panic { drop_counter: &DROP_COUNTER, value: true, index: 7 },
Panic { drop_counter, value: false, index: 1 },
Panic { drop_counter, value: false, index: 2 },
Panic { drop_counter, value: false, index: 3 },
Panic { drop_counter, value: false, index: 4 },
// here it panics while dropping the item with index==4
Panic { drop_counter, value: false, index: 5 },
Panic { drop_counter, value: true, index: 6 },
Panic { drop_counter, value: true, index: 7 },
];

let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
vec.dedup();
}));
let _ = catch_unwind(AssertUnwindSafe(|| vec.dedup())).unwrap_err();

assert_eq!(drop_counter.get(), 4);

let ok = vec.iter().zip(expected.iter()).all(|(x, y)| x.index == y.index);

Expand Down
2 changes: 1 addition & 1 deletion library/core/src/array/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ impl<T, const N: usize> Iterator for IntoIter<T, N> {
// SAFETY: Callers are only allowed to pass an index that is in bounds
// Additionally Self: TrustedRandomAccess is only implemented for T: Copy which means even
// multiple repeated reads of the same index would be safe and the
// values aree !Drop, thus won't suffer from double drops.
// values are !Drop, thus won't suffer from double drops.
unsafe { self.data.get_unchecked(self.alive.start + idx).assume_init_read() }
}
}
Expand Down
5 changes: 0 additions & 5 deletions library/core/src/iter/traits/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,6 @@ fn _assert_is_object_safe(_: &dyn Iterator<Item = ()>) {}
_Self = "std::string::String",
label = "`{Self}` is not an iterator; try calling `.chars()` or `.bytes()`"
),
on(
_Self = "[]",
label = "arrays do not yet implement `IntoIterator`; try using `std::array::IntoIter::new(arr)`",
note = "see <https://github.com/rust-lang/rust/pull/65819> for more details"
),
on(
_Self = "{integral}",
note = "if you want to iterate between `start` until a value `end`, use the exclusive range \
Expand Down
1 change: 1 addition & 0 deletions library/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@
#![feature(no_coverage)] // rust-lang/rust#84605
#![feature(int_error_matching)]
#![deny(unsafe_op_in_unsafe_fn)]
#![deny(or_patterns_back_compat)]

// allow using `core::` in intra-doc links
#[allow(unused_extern_crates)]
Expand Down
Loading

0 comments on commit f6a28aa

Please sign in to comment.