Skip to content

Commit

Permalink
Don't ICE on errors in function returning impl trait
Browse files Browse the repository at this point in the history
Instead, report the error.

This emits the errors on-demand, without special-casing `impl Trait`, so
it should catch all ICEs of this kind, including ones that haven't been
found yet.

Since the error is emitted during type-checking there is less info about
the error; see comments in the code for details.

- Add test case for -> impl Trait
- Add test for impl trait with alias
- Move EmitIgnoredResolutionErrors to rustdoc

This makes `fn typeck_item_bodies` public, which is not desired behavior.
That change should be removed once
rust-lang#74070 is merged.

- Don't visit nested closures twice
  • Loading branch information
jyn514 committed Jul 15, 2020
1 parent 14a8707 commit 768d6a4
Show file tree
Hide file tree
Showing 7 changed files with 162 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -955,7 +955,7 @@ where
val.fold_with(&mut FixupFolder { tcx })
}

fn typeck_tables_of<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> &ty::TypeckTables<'tcx> {
pub fn typeck_tables_of<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> &ty::TypeckTables<'tcx> {
let fallback = move || tcx.type_of(def_id.to_def_id());
typeck_tables_of_with_fallback(tcx, def_id, fallback)
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ extern crate rustc_middle;
pub mod expr_use_visitor;

mod astconv;
mod check;
pub mod check;
mod check_unused;
mod coherence;
mod collect;
Expand Down
78 changes: 78 additions & 0 deletions src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,15 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
override_queries: Some(|_sess, local_providers, external_providers| {
local_providers.lint_mod = |_, _| {};
external_providers.lint_mod = |_, _| {};
//let old_typeck = local_providers.typeck_tables_of;
local_providers.typeck_tables_of = move |tcx, def_id| {
let hir = tcx.hir();
let body = hir.body(hir.body_owned_by(hir.as_local_hir_id(def_id)));
debug!("visiting body for {:?}", def_id);
EmitIgnoredResolutionErrors::new(&tcx.sess).visit_body(body);
rustc_typeck::check::typeck_tables_of(tcx, def_id)
//DEFAULT_TYPECK.with(|typeck| typeck(tcx, def_id))
};
}),
registry: rustc_driver::diagnostics_registry(),
};
Expand Down Expand Up @@ -572,6 +581,75 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
})
}

use rustc_hir::def::Res;
use rustc_hir::{
intravisit::{NestedVisitorMap, Visitor},
Path,
};
use rustc_middle::hir::map::Map;

/*
thread_local!(static DEFAULT_TYPECK: for<'tcx> fn(rustc_middle::ty::TyCtxt<'tcx>, rustc_span::def_id::LocalDefId) -> &'tcx rustc_middle::ty::TypeckTables<'tcx> = {
let mut providers = rustc_middle::ty::query::Providers::default();
rustc_typeck::provide(&mut providers);
providers.typeck_tables_of
});
*/

/// Due to https://github.com/rust-lang/rust/pull/73566,
/// the name resolution pass may find errors that are never emitted.
/// If typeck is called after this happens, then we'll get an ICE:
/// 'Res::Error found but not reported'. To avoid this, emit the errors now.
struct EmitIgnoredResolutionErrors<'a> {
session: &'a Session,
}

impl<'a> EmitIgnoredResolutionErrors<'a> {
fn new(session: &'a Session) -> Self {
Self { session }
}
}

impl<'a> Visitor<'a> for EmitIgnoredResolutionErrors<'_> {
type Map = Map<'a>;

fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
// If we visit nested bodies, then we will report errors twice for e.g. nested closures
NestedVisitorMap::None
}

fn visit_path(&mut self, path: &'v Path<'v>, _id: HirId) {
log::debug!("visiting path {:?}", path);
if path.res == Res::Err {
// We have less context here than in rustc_resolve,
// so we can only emit the name and span.
// However we can give a hint that rustc_resolve will have more info.
// NOTE: this is a very rare case (only 4 out of several hundred thousand crates in a crater run)
// NOTE: so it's ok for it to be slow
let label = format!(
"could not resolve path `{}`",
path.segments
.iter()
.map(|segment| segment.ident.as_str().to_string())
.collect::<Vec<_>>()
.join("::")
);
let mut err = rustc_errors::struct_span_err!(
self.session,
path.span,
E0433,
"failed to resolve: {}",
label
);
err.span_label(path.span, label);
err.note("this error was originally ignored because you are running `rustdoc`");
err.note("try running again with `rustc` and you may get a more detailed error");
err.emit();
}
// NOTE: this does _not_ visit the path segments
}
}

/// `DefId` or parameter index (`ty::ParamTy.index`) of a synthetic type parameter
/// for `impl Trait` in argument position.
#[derive(Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)]
Expand Down
1 change: 1 addition & 0 deletions src/librustdoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ pub fn main() {
32_000_000 // 32MB on other platforms
};
rustc_driver::set_sigpipe_handler();
rustc_driver::install_ice_hook();
env_logger::init_from_env("RUSTDOC_LOG");
let res = std::thread::Builder::new()
.stack_size(thread_stack_size)
Expand Down
28 changes: 28 additions & 0 deletions src/test/rustdoc-ui/error-in-impl-trait.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// edition:2018
#![feature(type_alias_impl_trait)]

pub trait ValidTrait {}
type ImplTrait = impl ValidTrait;

/// This returns impl trait
pub fn g() -> impl ValidTrait {
error::_in::impl_trait()
//~^ ERROR failed to resolve
}

/// This returns impl trait, but using a type alias
pub fn h() -> ImplTrait {
error::_in::impl_trait::alias();
//~^ ERROR failed to resolve
(|| error::_in::impl_trait::alias::nested::closure())()
//~^ ERROR failed to resolve
}

/// This used to work with ResolveBodyWithLoop.
/// However now that we ignore type checking instead of modifying the function body,
/// the return type is seen as `impl Future<Output = u32>`, not a `u32`.
/// So it no longer allows errors in the function body.
pub async fn a() -> u32 {
error::_in::async_fn()
//~^ ERROR failed to resolve
}
39 changes: 39 additions & 0 deletions src/test/rustdoc-ui/error-in-impl-trait.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
error[E0433]: failed to resolve: could not resolve path `error::_in::impl_trait`
--> $DIR/error-in-impl-trait.rs:9:5
|
LL | error::_in::impl_trait()
| ^^^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::impl_trait`
|
= note: this error was originally ignored because you are running `rustdoc`
= note: try running again with `rustc` and you may get a more detailed error

error[E0433]: failed to resolve: could not resolve path `error::_in::impl_trait::alias`
--> $DIR/error-in-impl-trait.rs:15:5
|
LL | error::_in::impl_trait::alias();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::impl_trait::alias`
|
= note: this error was originally ignored because you are running `rustdoc`
= note: try running again with `rustc` and you may get a more detailed error

error[E0433]: failed to resolve: could not resolve path `error::_in::impl_trait::alias::nested::closure`
--> $DIR/error-in-impl-trait.rs:17:9
|
LL | (|| error::_in::impl_trait::alias::nested::closure())()
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::impl_trait::alias::nested::closure`
|
= note: this error was originally ignored because you are running `rustdoc`
= note: try running again with `rustc` and you may get a more detailed error

error[E0433]: failed to resolve: could not resolve path `error::_in::async_fn`
--> $DIR/error-in-impl-trait.rs:26:5
|
LL | error::_in::async_fn()
| ^^^^^^^^^^^^^^^^^^^^ could not resolve path `error::_in::async_fn`
|
= note: this error was originally ignored because you are running `rustdoc`
= note: try running again with `rustc` and you may get a more detailed error

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0433`.
14 changes: 14 additions & 0 deletions src/test/rustdoc/impl-trait-alias.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#![feature(type_alias_impl_trait)]

trait MyTrait {}
impl MyTrait for i32 {}

// @has impl_trait_alias/type.Foo.html 'Foo'
/// debug type
pub type Foo = impl MyTrait;

// @has impl_trait_alias/fn.foo.html 'foo'
/// debug function
pub fn foo() -> Foo {
1
}

0 comments on commit 768d6a4

Please sign in to comment.