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 8 pull requests #85720

Merged
merged 21 commits into from
May 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
f749d88
Disallow shadowing const parameters
FabianWolff May 19, 2021
e0c9719
Avoid a double drop in Vec::dedup if a destructor panics
SkiFire13 May 24, 2021
c9595fa
Make Vec::dedup panicking test actually detect double panics
SkiFire13 May 24, 2021
8d954fa
Remove arrays/IntoIterator message from Iterator trait.
m-ou-se May 25, 2021
3ed90e2
fix matches! and assert_matches! on edition 2021
May 25, 2021
0baf898
Remove num_as_ne_bytes feature
hch12907 May 25, 2021
824c743
add regression test
May 25, 2021
d14dd9f
emit diagnostic after post-monomorphization errors
lqd May 16, 2021
6f61456
add test for issue 85155 and similar
lqd May 24, 2021
b083541
Handle `unsafe_op_in_unsafe_fn` properly in THIR unsafeck
LeSeulArtichaut May 24, 2021
f7916b4
Fix `unused_unsafe` in THIR unsafeck
LeSeulArtichaut May 24, 2021
f9e08cd
Run THIR unsafeck on `unsafe_op_in_unsafe_fn` test
LeSeulArtichaut May 24, 2021
d1b69cf
Fix typo in core::array::IntoIter comment
BlackHoleFox May 26, 2021
69c78a9
Rollup merge of #85478 - FabianWolff:issue-85348, r=petrochenkov
Dylan-DPC May 26, 2021
27899e3
Rollup merge of #85625 - SkiFire13:fix-85613-vec-dedup-drop-panics, r…
Dylan-DPC May 26, 2021
f5c5cca
Rollup merge of #85627 - LeSeulArtichaut:thir-unsafe-fn-lint, r=nikom…
Dylan-DPC May 26, 2021
4b0014e
Rollup merge of #85633 - lqd:stackless_span_stacks, r=oli-obk
Dylan-DPC May 26, 2021
12ab323
Rollup merge of #85670 - m-ou-se:array-intoiter-1, r=scottmcm
Dylan-DPC May 26, 2021
3c2a709
Rollup merge of #85678 - lukas-code:matches2021, r=dtolnay
Dylan-DPC May 26, 2021
f3b10dd
Rollup merge of #85679 - hch12907:master, r=Mark-Simulacrum
Dylan-DPC May 26, 2021
9ee87c7
Rollup merge of #85712 - BlackHoleFox:fix-iter-typo, r=jyn514
Dylan-DPC May 26, 2021
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
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