Skip to content

[WIP] Tracking all the unsolved variables that was assigned ! type because of fallback #74535

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

Closed
wants to merge 6 commits into from
Closed
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
8 changes: 8 additions & 0 deletions compiler/rustc_session/src/lint/builtin.rs
Original file line number Diff line number Diff line change
@@ -2493,6 +2493,13 @@ declare_lint! {
"using only a subset of a register for inline asm inputs",
}

declare_lint! {
//TODO: Add explanation.
pub FALL_BACK_TO_NEVER_TYPE,
Deny,
"Unresolved variable might fall back to never_type `!`"
}

declare_lint! {
/// The `unsafe_op_in_unsafe_fn` lint detects unsafe operations in unsafe
/// functions without an explicit unsafe block. This lint only works on
@@ -2680,6 +2687,7 @@ declare_lint_pass! {
SAFE_PACKED_BORROWS,
PATTERNS_IN_FNS_WITHOUT_BODY,
LATE_BOUND_LIFETIME_ARGUMENTS,
FALL_BACK_TO_NEVER_TYPE,
ORDER_DEPENDENT_TRAIT_OBJECTS,
COHERENCE_LEAK_CHECK,
DEPRECATED,
4 changes: 4 additions & 0 deletions compiler/rustc_typeck/src/check/expr.rs
Original file line number Diff line number Diff line change
@@ -214,6 +214,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// Combine the diverging and has_error flags.
self.diverges.set(self.diverges.get() | old_diverges);
self.has_errors.set(self.has_errors.get() | old_has_errors);
if self.diverges.get().is_always() {
self.dead_nodes.borrow_mut().insert(expr.hir_id);
debug!("expr with HIR id {:?} is dead on exit", expr.hir_id);
}

debug!("type of {} is...", self.tcx.hir().node_to_string(expr.hir_id));
debug!("... {:?}, expected is {:?}", ty, expected);
10 changes: 10 additions & 0 deletions compiler/rustc_typeck/src/check/fn_ctxt.rs
Original file line number Diff line number Diff line change
@@ -143,6 +143,10 @@ pub struct FnCtxt<'a, 'tcx> {
/// the diverges flag is set to something other than `Maybe`.
pub(super) diverges: Cell<Diverges>,

/// This keeps track of the dead nodes. We use this to determine
/// if there are live nodes with the diverging fallback for linting.
pub(super) dead_nodes: RefCell<FxHashSet<hir::HirId>>,

/// Whether any child nodes have any type errors.
pub(super) has_errors: Cell<bool>,

@@ -175,6 +179,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
by_id: Default::default(),
}),
inh,
dead_nodes: RefCell::new(FxHashSet::default()),
}
}

@@ -1955,6 +1960,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.diverges.set(prev_diverges);
}

if self.diverges.get().is_always() {
self.dead_nodes.borrow_mut().insert(blk.hir_id);
debug!("expr with HIR id {:?} is dead on exit", blk.hir_id);
}

let mut ty = ctxt.coerce.unwrap().complete(self);

if self.has_errors.get() || ty.references_error() {
16 changes: 14 additions & 2 deletions compiler/rustc_typeck/src/check/mod.rs
Original file line number Diff line number Diff line change
@@ -552,10 +552,12 @@ fn typeck_with_fallback<'tcx>(
fcx.select_obligations_where_possible(false, |_| {});
let mut fallback_has_occurred = false;

let unsolved_variables = fcx.unsolved_variables();

// We do fallback in two passes, to try to generate
// better error messages.
// The first time, we do *not* replace opaque types.
for ty in &fcx.unsolved_variables() {
for ty in &unsolved_variables {
fallback_has_occurred |= fcx.fallback_if_possible(ty, FallbackMode::NoOpaque);
}
// We now see if we can make progress. This might
@@ -583,6 +585,16 @@ fn typeck_with_fallback<'tcx>(
// refer to opaque types.
fcx.select_obligations_where_possible(fallback_has_occurred, |_| {});

// We run through the list of `unsolved_variables` gathered earlier and
// check if there are any that are marked `Diverging` at this point. if
// so, this would imply, that they were assigned Divergent type because
// of fallback. Any type in `unsolved_variables` that is now `!`, is `!`
// as a result of fallback.
let mut from_diverging_fallback = unsolved_variables;
let diverging_default = fcx.tcx.mk_diverging_default();
from_diverging_fallback
.retain(|ty| fcx.infcx.resolve_vars_if_possible(ty) == diverging_default);

// We now run fallback again, but this time we allow it to replace
// unconstrained opaque type variables, in addition to performing
// other kinds of fallback.
@@ -616,7 +628,7 @@ fn typeck_with_fallback<'tcx>(
fcx.regionck_expr(body);
}

fcx.resolve_type_vars_in_body(body)
fcx.resolve_type_vars_in_body(body, &from_diverging_fallback)
});

// Consistency check our TypeckResults instance can hold all ItemLocalIds
43 changes: 40 additions & 3 deletions compiler/rustc_typeck/src/check/writeback.rs
Original file line number Diff line number Diff line change
@@ -34,6 +34,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pub fn resolve_type_vars_in_body(
&self,
body: &'tcx hir::Body<'tcx>,
from_diverging_fallback: &Vec<Ty<'tcx>>,
) -> &'tcx ty::TypeckResults<'tcx> {
let item_id = self.tcx.hir().body_owner(body.id());
let item_def_id = self.tcx.hir().local_def_id(item_id);
@@ -43,7 +44,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let rustc_dump_user_substs =
self.tcx.has_attr(item_def_id.to_def_id(), sym::rustc_dump_user_substs);

let mut wbcx = WritebackCx::new(self, body, rustc_dump_user_substs);
let mut wbcx =
WritebackCx::new(self, body, rustc_dump_user_substs, from_diverging_fallback);
for param in body.params {
wbcx.visit_node_id(param.pat.span, param.hir_id);
}
@@ -100,13 +102,19 @@ struct WritebackCx<'cx, 'tcx> {
body: &'tcx hir::Body<'tcx>,

rustc_dump_user_substs: bool,

/// List of type variables which became the never type `!`
/// as a result of fallback.
/// This is used to issue lints and warnings for the user.
from_diverging_fallback: &'cx Vec<Ty<'tcx>>,
}

impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
fn new(
fcx: &'cx FnCtxt<'cx, 'tcx>,
body: &'tcx hir::Body<'tcx>,
rustc_dump_user_substs: bool,
from_diverging_fallback: &'cx Vec<Ty<'tcx>>,
) -> WritebackCx<'cx, 'tcx> {
let owner = body.id().hir_id.owner;

@@ -115,6 +123,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
typeck_results: ty::TypeckResults::new(owner),
body,
rustc_dump_user_substs,
from_diverging_fallback,
}
}

@@ -521,8 +530,36 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
self.visit_adjustments(span, hir_id);

// Resolve the type of the node with id `node_id`
let n_ty = self.fcx.node_ty(hir_id);
let n_ty = self.resolve(&n_ty, &span);
let n_ty_original = self.fcx.node_ty(hir_id);
let n_ty = self.resolve(&n_ty_original, &span);

debug!("visit_node_id: {:?}", self.from_diverging_fallback);
// check whether the node type contains any of the variables that
// became `!` as a result of type fallback but they are not part of the
// dead nodes. if so, warn. Note that, we concern ourselves with only
// the `n_ty_original` and don't `walk()` the subparts of a type. So, for a
// variable like `Foo<Bar<Bas<...<N>>>>` even if `N` is diverging type,
// we will not generate a warning. This might be okay as sometimes we may
// have things like `Result<i32, T> where even though `T` is diverging,
// it might never be used and warning would be confusing for the user.
if !self.from_diverging_fallback.is_empty() {
debug!("hir_id:{}", &hir_id);
debug!("n_ty_original:{}", &n_ty_original);
if !self.fcx.dead_nodes.borrow().contains(&hir_id)
&& self.from_diverging_fallback.contains(&n_ty_original)
{
self.tcx()
.struct_span_lint_hir(
rustc_session::lint::builtin::FALL_BACK_TO_NEVER_TYPE,
hir_id,
span,
|lint| {
lint.build(&format!("resulted from diverging fallback: {:?}", n_ty)).emit()
}
);
}
}

self.write_ty_to_typeck_results(hir_id, n_ty);
debug!("node {:?} has type {:?}", hir_id, n_ty);

11 changes: 11 additions & 0 deletions src/test/ui/never_type/never_type_false_warning_unreachable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#![deny(fall_back_to_never_type)]

macro_rules! unreachable1 {
() => {{ panic!("internal error: entered unreachable code") }};
}

fn get_unchecked() {
unreachable1!();
}

fn main() {}
20 changes: 20 additions & 0 deletions src/test/ui/never_type/never_type_lint.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#![feature(never_type_fallback)]

fn make_unit() {
}

fn unconstrained_return<T>() ->T {
unsafe {
let make_unit_fn: fn() = make_unit;
let ffi: fn() -> T = std::mem::transmute(make_unit_fn);
ffi()
}
}

fn main() {
let _ = if true {
unconstrained_return()
} else {
panic!()
};
}