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

Post-monomorphization errors traces MVP #85633

Merged
merged 2 commits into from
May 26, 2021
Merged
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
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Auxiliary crate used for testing post-monomorphization errors cross-crate.
// It duplicates the setup used in `stdarch` to validate its intrinsics' const arguments.

struct ValidateConstImm<const IMM: i32, const MIN: i32, const MAX: i32>;
impl<const IMM: i32, const MIN: i32, const MAX: i32> ValidateConstImm<IMM, MIN, MAX> {
pub(crate) const VALID: () = {
let _ = 1 / ((IMM >= MIN && IMM <= MAX) as usize);
};
}

macro_rules! static_assert_imm1 {
($imm:ident) => {
let _ = $crate::ValidateConstImm::<$imm, 0, { (1 << 1) - 1 }>::VALID;
};
}

// This function triggers an error whenever the const argument does not fit in 1-bit.
pub fn stdarch_intrinsic<const IMM1: i32>() {
static_assert_imm1!(IMM1);
}
21 changes: 21 additions & 0 deletions src/test/ui/consts/const-eval/issue-85155.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// This is a test with a setup similar to issue 85155, which triggers a const eval error: a const
// argument value is outside the range expected by the `stdarch` intrinsic.
//
// It's not the exact code mentioned in that issue because it depends both on `stdarch` intrinsics
// only available on x64, and internal implementation details of `stdarch`. But mostly because these
// are not important to trigger the diagnostics issue: it's specifically about the lack of context
// in the diagnostics of post-monomorphization errors (PMEs) for consts, happening in a dependency.
// Therefore, its setup is reproduced with an aux crate, which will similarly trigger a PME
// depending on the const argument value, like the `stdarch` intrinsics would.
//
// aux-build: post_monomorphization_error.rs
// build-fail: this is a post-monomorphization error, it passes check runs and requires building
// to actually fail.

extern crate post_monomorphization_error;

fn main() {
// This function triggers a PME whenever the const argument does not fit in 1-bit.
post_monomorphization_error::stdarch_intrinsic::<2>();
//~^ NOTE the above error was encountered while instantiating
}
15 changes: 15 additions & 0 deletions src/test/ui/consts/const-eval/issue-85155.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error[E0080]: evaluation of constant value failed
--> $DIR/auxiliary/post_monomorphization_error.rs:7:17
|
LL | let _ = 1 / ((IMM >= MIN && IMM <= MAX) as usize);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempt to divide `1_usize` by zero

note: the above error was encountered while instantiating `fn stdarch_intrinsic::<2_i32>`
--> $DIR/issue-85155.rs:19:5
|
LL | post_monomorphization_error::stdarch_intrinsic::<2>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0080`.